|
| 1 | +- 2018-01-17 |
| 2 | +- RFC PR: 0297 |
| 3 | +- Ember Issue: https://github.com/emberjs/ember.js/issues/16231 |
| 4 | + |
| 5 | +# Deprecation of Ember.Logger |
| 6 | + |
| 7 | +## Summary |
| 8 | + |
| 9 | +This RFC recommends the deprecation and eventual removal of `Ember.Logger`. |
| 10 | + |
| 11 | +## Motivation |
| 12 | + |
| 13 | +There are a variety of features of Ember designed to support old browsers, |
| 14 | +features that are no longer needed. `Ember.Logger` came into being because |
| 15 | +the browser support for the console was inconsistent. In some browsers, |
| 16 | +like Internet Explorer 9, the console only existed when the developer tools |
| 17 | +panel was open, which caused null references and program crashes when run |
| 18 | +with the console closed. `Ember.Logger` provided methods that would route to |
| 19 | +the console when it was available. |
| 20 | + |
| 21 | +With Ember 3.x, Ember no longer supports these older browsers, and hence this |
| 22 | +feature no longer serves a purpose. Removing it will make Ember smaller and |
| 23 | +lighter. |
| 24 | + |
| 25 | +## Detailed design |
| 26 | + |
| 27 | +For the most part, this is a 1:1 substitution of the global `console` object |
| 28 | +for `Ember.Logger`. However, Node only added support for `console.debug` in |
| 29 | +Node version 9. If we wish to support earlier versions of Node, our codemod |
| 30 | +will need to use `console.log`, rather than `console.debug`, as the |
| 31 | +replacement for `Logger.debug`. For users who don't care about Node or are |
| 32 | +specifying Node version 9 as their minimum, we could use `console.debug`. |
| 33 | + |
| 34 | +### Within the framework |
| 35 | + |
| 36 | +Remove the following direct uses of `Ember.Logger` from the ember.js and |
| 37 | +ember-data projects: |
| 38 | + |
| 39 | +* `ember-debug`: |
| 40 | + * deprecate (`ember-debug\lib\deprecate.js`) - `Logger.warn` |
| 41 | + * debug (`ember-debug\lib\index.js`) - `Logger.info` |
| 42 | + * warn (`ember-debug\lib\warn.js`) - `Logger.warn` |
| 43 | +* `ember-routing` (`ember-routing\lib\system\router.js`): |
| 44 | + * transitioned to - `Logger.log` |
| 45 | + * preparing to transition to - `Logger.log` |
| 46 | + * intermediate-transitioned to - `Logger.log` |
| 47 | +* `ember-testing`: |
| 48 | + * Testing paused (`ember-testing\lib\helpers\pause_test.js`) - `Logger.info` |
| 49 | + * Catch-all handler (`ember-testing\lib\test\adapter.js`) - `Logger.error` |
| 50 | +* `ember-data`: |
| 51 | + * `tests\test-helper.js`- `Logger.log` |
| 52 | + |
| 53 | +Adjust all test code that redirects logging and sets it back: |
| 54 | + |
| 55 | +* `ember\tests\routing\basic_test.js` (adjust) |
| 56 | +* `ember-application\tests\system\dependency_injection\default_resolver_test.js` (adjust) |
| 57 | +* `ember-application\tests\system\logging_test.js` (remove?) |
| 58 | +* `ember-glimmer\tests\integration\helpers\log-test.js` (remove?) |
| 59 | + |
| 60 | +Note: None of the uses of `Ember.Logger` in `ember.js` or `ember-data` involve |
| 61 | +`Ember.debug`, so that issue doesn't affect the Ember.js code directly. |
| 62 | + |
| 63 | +Add deprecation warnings to the implementation: `ember-console\lib\index.js`. |
| 64 | +Bear in mind that `Ember.deprecate` in `ember-debug` currently calls |
| 65 | +`Logger.warn`, so the `ember-debug` code should be changed _first_ or adding |
| 66 | +the deprecation warning will create a deep recursion. |
| 67 | + |
| 68 | +The `Ember.assert`, `Ember.warn`, `Ember.info`, `Ember.debug`, and |
| 69 | +`Ember.deprecate` methods suppress their output on production builds. |
| 70 | +However, they are suppressing them in the `ember-debug` module, which |
| 71 | +currently consumes `Ember.Logger`, _not_ by `Ember.Logger` itself. Hence, |
| 72 | +replacing calls to `Ember.Logger` with direct calls to the console will not |
| 73 | +affect this behavior. |
| 74 | + |
| 75 | +### Codemod |
| 76 | + |
| 77 | +Provide a codemod that developers can use to switch references to the methods |
| 78 | +of `Ember.Logger` to use the corresponding `console` methods instead. |
| 79 | + |
| 80 | +The codemod will need to replace `Ember.Logger.debug` calls with `(console.debug || |
| 81 | +console.log)(<arguments>)` or something similar to take in stride the situation |
| 82 | +where console.debug isn't defined. |
| 83 | + |
| 84 | +I will definitely need help here. |
| 85 | + |
| 86 | +### Add-On Developers |
| 87 | + |
| 88 | +The following high-impact add-ons (9 or 10 or a * on EmberObserver) use |
| 89 | +`Ember.Logger` and should probably be given an early heads-up to adjust |
| 90 | +their code to use `console` before this RFC is implemented. This will limit |
| 91 | +the level of pain that their users experience when the deprecation is released. |
| 92 | + |
| 93 | +In the order of their number of references to `Ember.Logger`: |
| 94 | + |
| 95 | +* `ember-concurrency` (15) |
| 96 | +* `ember-cli-deprecation-workflow` (9) |
| 97 | +* `ember-stripe-service` (9) |
| 98 | +* `semantic-ui-ember` (7) |
| 99 | +* `ember-resolver` (6) |
| 100 | +* `ember-cli-page-object` (4) |
| 101 | +* `ember-cli-sentry` (3) |
| 102 | +* `ember-islands` (3) |
| 103 | +* `ember-states` (3) |
| 104 | +* `ember-cli-pagination` (2) |
| 105 | +* `ember-cli-clipboard` (1) |
| 106 | +* `ember-cli-fastboot` (1) |
| 107 | +* `ember-elsewhere` (1) |
| 108 | +* `ember-i18n` (1) |
| 109 | +* `ember-simple-auth-token` (1) |
| 110 | +* `ember-svg-jar` (1) |
| 111 | +* `liquid-fire` (1) |
| 112 | + |
| 113 | +For details, see https://emberobserver.com/code-search?codeQuery=Ember.Logger. |
| 114 | + |
| 115 | +## How we teach this |
| 116 | + |
| 117 | +### Communication of change |
| 118 | + |
| 119 | +We need to inform users that `Ember.Logger` will be deprecated and in what |
| 120 | +release it will occur. |
| 121 | + |
| 122 | +### Official code bases and documentation |
| 123 | + |
| 124 | +We do not currently actively teach the use of `Ember.Logger`. We will need to |
| 125 | +remove any passing references to `Ember.Logger` from the Ember guides |
| 126 | +from the Super Rentals tutorial, and anywhere else it appears on the website. |
| 127 | + |
| 128 | +Once it is gone from the code, we also need to verify it no longer appears in |
| 129 | +the API listings. |
| 130 | + |
| 131 | +We must provide an entry in the deprecation guide for this change: |
| 132 | +* offering instruction for using the codemod to perform the change automatically |
| 133 | +with before and after code samples. |
| 134 | +* describing the issue with using console.debug on node versions |
| 135 | +earlier than Node 9 and what provision the codemod has made to deal with it. |
| 136 | +* describing alternative ways of dealing with eslint's `no-console` messages. |
| 137 | + |
| 138 | +## Drawbacks |
| 139 | + |
| 140 | +191 add-ons in Ember Inspector are using `Ember.Logger`. It has been there and |
| 141 | +documented for a long time. So this deprecation will cause some level of change |
| 142 | +on many projects. |
| 143 | + |
| 144 | +This, of course, can be said for almost any deprecation, and Ember's |
| 145 | +disciplined approach to deprecation has been repeatedly shown to ease things. |
| 146 | +Providing a codemod to replace `Ember.Logger` calls with the corresponding |
| 147 | +console calls should make this transition relatively painless. Also, only |
| 148 | +twenty of those add-ons have more than six references to `Ember.Logger`. |
| 149 | +If this is characteristic of the user base, the level of effort to make |
| 150 | +the change, even by hand, should be very small for most users. |
| 151 | + |
| 152 | +Those using `Logger.debug` as something different from `Logger.log` may have |
| 153 | +at least a theoretical concern. Under the covers `Logger.debug` only calls |
| 154 | +`console.debug` if it exists, calling `console.log` otherwise. The only |
| 155 | +platform where the difference between the two is visible in the console is on |
| 156 | +Safari. We can encourage folks with a tangible, practical concern about this to |
| 157 | +speak up during the comment period, but I don't anticipate this will have much |
| 158 | +impact. |
| 159 | + |
| 160 | +## Alternatives |
| 161 | + |
| 162 | +1. Leave things as they are, perhaps providing an `@ember/console` module |
| 163 | +interface. |
| 164 | + |
| 165 | +2. Extract `Ember.Logger` into its own (tiny) `@ember/console` package as |
| 166 | +a shim for users. |
| 167 | + |
| 168 | +## Unresolved questions |
| 169 | + |
| 170 | +None at this point. The answers from prior drafts have been promoted into the text. |
0 commit comments