|
| 1 | +--- |
| 2 | +stage: accepted |
| 3 | +start-date: 2024-02-22T00:00:00.000Z |
| 4 | +release-date: # In format YYYY-MM-DDT00:00:00.000Z |
| 5 | +release-versions: |
| 6 | +teams: # delete teams that aren't relevant |
| 7 | + - cli |
| 8 | + - data |
| 9 | + - framework |
| 10 | + - learning |
| 11 | + - typescript |
| 12 | +prs: |
| 13 | + accepted: https://github.com/emberjs/rfcs/pull/1009 |
| 14 | +project-link: |
| 15 | +suite: |
| 16 | +--- |
| 17 | + |
| 18 | +<!--- |
| 19 | +Directions for above: |
| 20 | +
|
| 21 | +stage: Leave as is |
| 22 | +start-date: Fill in with today's date, 2032-12-01T00:00:00.000Z |
| 23 | +release-date: Leave as is |
| 24 | +release-versions: Leave as is |
| 25 | +teams: Include only the [team(s)](README.md#relevant-teams) for which this RFC applies |
| 26 | +prs: |
| 27 | + accepted: Fill this in with the URL for the Proposal RFC PR |
| 28 | +project-link: Leave as is |
| 29 | +suite: Leave as is |
| 30 | +--> |
| 31 | + |
| 32 | +# Move the deprecation workflow library to be installed in apps by default |
| 33 | + |
| 34 | +## Summary |
| 35 | + |
| 36 | +Historically, folks have benefitted from [ember-cli-deprecation-workflow](https://github.com/mixonic/ember-cli-deprecation-workflow). This behavior is _so useful_, that it should be built in to folks applications by default. |
| 37 | + |
| 38 | +## Motivation |
| 39 | + |
| 40 | +Everyone needs a deprecation-workflow, and yet `ember-cli-deprecation-workflow` is not part of the default blueprint. |
| 41 | + |
| 42 | +This RFC proposes how we can ship deprecation workflow handling behavior in apps by default, which may give us a blessed path for better integrating with build time deprecations as well (though that is not the focus of this RFC). |
| 43 | + |
| 44 | + |
| 45 | +## Detailed design |
| 46 | + |
| 47 | + |
| 48 | +Have `ember-cli-deprecation-workflow` installed by default. |
| 49 | + |
| 50 | +1. applications must have `@embroider/macros` installed by default. |
| 51 | +2. the app.js or app.ts can conditionally import a file which sets up the deprecation workflow |
| 52 | + ```diff app/app.js |
| 53 | + import Application from '@ember/application'; |
| 54 | + + import { importSync, isDevelopingApp, macroCondition } from '@embroider/macros'; |
| 55 | + |
| 56 | + import loadInitializers from 'ember-load-initializers'; |
| 57 | + import Resolver from 'ember-resolver'; |
| 58 | + import config from 'test-app/config/environment'; |
| 59 | + |
| 60 | + + if (macroCondition(isDevelopingApp())) { |
| 61 | + + importSync('./deprecation-workflow'); |
| 62 | + + } |
| 63 | + |
| 64 | + export default class App extends Application { |
| 65 | + modulePrefix = config.modulePrefix; |
| 66 | + podModulePrefix = config.podModulePrefix; |
| 67 | + Resolver = Resolver; |
| 68 | + } |
| 69 | + |
| 70 | + loadInitializers(App, config.modulePrefix); |
| 71 | + ``` |
| 72 | +3. then in `app/deprecation-workflow.js` would use the already public API, |
| 73 | + ```js |
| 74 | + import setupDeprecationWorkflow from 'ember-cli-deprecation-workflow'; |
| 75 | + |
| 76 | + setupDeprecationWorkflow({ |
| 77 | + /** |
| 78 | + false by default, but if a developer / team wants to be more aggressive about being proactive with |
| 79 | + handling their deprecations, this should be set to "true" |
| 80 | + */ |
| 81 | + throwOnUnhandled: false, |
| 82 | + handlers: [ |
| 83 | + /* ... handlers ... */ |
| 84 | + ] |
| 85 | + }); |
| 86 | + ``` |
| 87 | + |
| 88 | + |
| 89 | +This follows the README of [ember-cli-deprecation-workflow](https://github.com/ember-cli/ember-cli-deprecation-workflow?tab=readme-ov-file#getting-started). |
| 90 | + |
| 91 | + |
| 92 | + |
| 93 | +## How we teach this |
| 94 | + |
| 95 | +We'd want to add a new section in the guides under [`Application Concerns`](https://guides.emberjs.com/release/applications/) that talks about deprecations, how and how to work through those deprecations. |
| 96 | + |
| 97 | +All of this content already exists using a similar strategy as above, here, [under "Configuring Ember"](https://guides.emberjs.com/release/configuring-ember/handling-deprecations/#toc_deprecation-workflow), and also walks through how to use `ember-cli-deprecation-workflow`. |
| 98 | + |
| 99 | +This README of [ember-cli-deprecation-workflow](https://github.com/ember-cli/ember-cli-deprecation-workflow?tab=readme-ov-file#getting-started) also explains, in detail, how to use this tool / workflow, and that content can be copied in to the guides. |
| 100 | + |
| 101 | +## Drawbacks |
| 102 | + |
| 103 | +For older projects, this could be _a_ migration. But as it is additional blueprint boilerplate, it is optional. |
| 104 | + |
| 105 | +## Alternatives |
| 106 | + |
| 107 | +There are only a few features of `ember-cli-deprecation-workflow` that we need to worry about: |
| 108 | +- enabled or not - do we check deprecations at all, or ignore everything (current default) |
| 109 | +- `throwOnUnhandled` - this is the most aggressive way to stay on top of your deprecations, but can be frustrating for folks who may not be willing to fix things in `node_modules` when new deprecations are introduced. |
| 110 | + |
| 111 | +- `window.flushDeprecations()` - prints the list of deprecations encountered since the last page refresh |
| 112 | +- Matchers - a fuzzier way to match deprecation messages rather than strictly matching on the deprecation id (sometimes deprecation messages have information about surrounding / relevant context, and these could be used to more fine-grainedly work through large-in-numbers deprecations) |
| 113 | +- Logging / Ignoring / Throwing - when encountering a matched deprecation (whether by id or by regex, how should it be handled?) |
| 114 | + |
| 115 | + |
| 116 | +However, folks can get a basic deprecation-handling workflow going in their apps without the above features, |
| 117 | + |
| 118 | +1. applications must have `@embroider/macros` installed by default. |
| 119 | +2. the app.js or app.ts can conditionally import a file which sets up the deprecation workflow |
| 120 | + ```diff app/app.js |
| 121 | + import Application from '@ember/application'; |
| 122 | + + import { importSync, isDevelopingApp, macroCondition } from '@embroider/macros'; |
| 123 | + |
| 124 | + import loadInitializers from 'ember-load-initializers'; |
| 125 | + import Resolver from 'ember-resolver'; |
| 126 | + import config from 'test-app/config/environment'; |
| 127 | + |
| 128 | + + if (macroCondition(isDevelopingApp())) { |
| 129 | + + importSync('./deprecation-workflow'); |
| 130 | + + } |
| 131 | + |
| 132 | + export default class App extends Application { |
| 133 | + modulePrefix = config.modulePrefix; |
| 134 | + podModulePrefix = config.podModulePrefix; |
| 135 | + Resolver = Resolver; |
| 136 | + } |
| 137 | + |
| 138 | + loadInitializers(App, config.modulePrefix); |
| 139 | + ``` |
| 140 | + this conditional import is now easily customizable for folks in their apps, so they could opt to _not_ strip deprecation messages in production, and see where deprecated code is being hit by users (reported via Sentry, BugSnag, or some other reporting tool) -- which may be handy for folks who have a less-than-perfect test suite (tests being the only current way to automatically detect where deprecated code lives). |
| 141 | +3. the `app/deprecation-workflow.js` would use the already public API, [`registerDeprecationHandler`](https://api.emberjs.com/ember/5.6/functions/@ember%2Fdebug/registerDeprecationHandler) |
| 142 | + ```ts |
| 143 | + import { registerDeprecationHandler } from '@ember/debug'; |
| 144 | + |
| 145 | + import config from '<app-moduleName>/config/environment'; |
| 146 | + |
| 147 | + const SHOULD_THROW = config.environment !== 'production'; |
| 148 | + const SILENCED_DEPRECATIONS: string[] = [ |
| 149 | + // Add ids of deprecations you temporarily want to silence here. |
| 150 | + ]; |
| 151 | + |
| 152 | + registerDeprecationHandler((message, options, next) => { |
| 153 | + if (!options) { |
| 154 | + console.error('Missing options'); |
| 155 | + throw new Error(message); |
| 156 | + } |
| 157 | + |
| 158 | + if (SILENCED_DEPRECATIONS.includes(options.id)) { |
| 159 | + return; |
| 160 | + } else if (SHOULD_THROW) { |
| 161 | + throw new Error(message); |
| 162 | + } |
| 163 | + |
| 164 | + next(message, options); |
| 165 | + }); |
| 166 | + ``` |
| 167 | + |
| 168 | + |
| 169 | +This simple implementation of deprecation workflow may work for libraries' test-apps, but it is not as robust as what `ember-cli-deprecation-workflow` offers, per the above-listed set of features that folks are used to. |
| 170 | + |
| 171 | +To get all of those features from `ember-cli-deprecation-workflow`, we could define a function, `setupDeprecationWorkflow`, taken from the [Modernization PR on ember-cli-deprecation-workflow](https://github.com/mixonic/ember-cli-deprecation-workflow/pull/159), this is what the deprecation-workflow file could look like: |
| 172 | + |
| 173 | +<details><summary>ember-cli-deprecation-workflow/index.js</summary> |
| 174 | + |
| 175 | +```js |
| 176 | +import { registerDeprecationHandler } from '@ember/debug'; |
| 177 | + |
| 178 | +const LOG_LIMIT = 100; |
| 179 | + |
| 180 | +export default function setupDeprecationWorkflow(config) { |
| 181 | + self.deprecationWorkflow = self.deprecationWorkflow || {}; |
| 182 | + self.deprecationWorkflow.deprecationLog = { |
| 183 | + messages: {}, |
| 184 | + }; |
| 185 | + |
| 186 | + registerDeprecationHandler((message, options, next) => |
| 187 | + handleDeprecationWorkflow(config, message, options, next), |
| 188 | + ); |
| 189 | + |
| 190 | + registerDeprecationHandler(deprecationCollector); |
| 191 | + |
| 192 | + self.deprecationWorkflow.flushDeprecations = flushDeprecations; |
| 193 | +} |
| 194 | + |
| 195 | +let preamble = `import setupDeprecationWorkflow from 'ember-cli-deprecation-workflow'; |
| 196 | + |
| 197 | +setupDeprecationWorkflow({ |
| 198 | + workflow: [ |
| 199 | +`; |
| 200 | + |
| 201 | +let postamble = ` ] |
| 202 | +});`; |
| 203 | + |
| 204 | +export function detectWorkflow(config, message, options) { |
| 205 | + if (!config || !config.workflow) { |
| 206 | + return; |
| 207 | + } |
| 208 | + |
| 209 | + let i, workflow, matcher, idMatcher; |
| 210 | + for (i = 0; i < config.workflow.length; i++) { |
| 211 | + workflow = config.workflow[i]; |
| 212 | + matcher = workflow.matchMessage; |
| 213 | + idMatcher = workflow.matchId; |
| 214 | + |
| 215 | + if (typeof idMatcher === 'string' && options && idMatcher === options.id) { |
| 216 | + return workflow; |
| 217 | + } else if (typeof matcher === 'string' && matcher === message) { |
| 218 | + return workflow; |
| 219 | + } else if (matcher instanceof RegExp && matcher.exec(message)) { |
| 220 | + return workflow; |
| 221 | + } |
| 222 | + } |
| 223 | +} |
| 224 | + |
| 225 | +export function flushDeprecations() { |
| 226 | + let messages = self.deprecationWorkflow.deprecationLog.messages; |
| 227 | + let logs = []; |
| 228 | + |
| 229 | + for (let message in messages) { |
| 230 | + logs.push(messages[message]); |
| 231 | + } |
| 232 | + |
| 233 | + let deprecations = logs.join(',\n') + '\n'; |
| 234 | + |
| 235 | + return preamble + deprecations + postamble; |
| 236 | +} |
| 237 | + |
| 238 | +export function handleDeprecationWorkflow(config, message, options, next) { |
| 239 | + let matchingWorkflow = detectWorkflow(config, message, options); |
| 240 | + if (!matchingWorkflow) { |
| 241 | + if (config && config.throwOnUnhandled) { |
| 242 | + throw new Error(message); |
| 243 | + } else { |
| 244 | + next(message, options); |
| 245 | + } |
| 246 | + } else { |
| 247 | + switch (matchingWorkflow.handler) { |
| 248 | + case 'silence': |
| 249 | + // no-op |
| 250 | + break; |
| 251 | + case 'log': { |
| 252 | + let key = (options && options.id) || message; |
| 253 | + |
| 254 | + if (!self.deprecationWorkflow.logCounts) { |
| 255 | + self.deprecationWorkflow.logCounts = {}; |
| 256 | + } |
| 257 | + |
| 258 | + let count = self.deprecationWorkflow.logCounts[key] || 0; |
| 259 | + self.deprecationWorkflow.logCounts[key] = ++count; |
| 260 | + |
| 261 | + if (count <= LOG_LIMIT) { |
| 262 | + console.warn('DEPRECATION: ' + message); |
| 263 | + if (count === LOG_LIMIT) { |
| 264 | + console.warn( |
| 265 | + 'To avoid console overflow, this deprecation will not be logged any more in this run.', |
| 266 | + ); |
| 267 | + } |
| 268 | + } |
| 269 | + |
| 270 | + break; |
| 271 | + } |
| 272 | + case 'throw': |
| 273 | + throw new Error(message); |
| 274 | + default: |
| 275 | + next(message, options); |
| 276 | + break; |
| 277 | + } |
| 278 | + } |
| 279 | +} |
| 280 | + |
| 281 | +export function deprecationCollector(message, options, next) { |
| 282 | + let key = (options && options.id) || message; |
| 283 | + let matchKey = options && key === options.id ? 'matchId' : 'matchMessage'; |
| 284 | + |
| 285 | + self.deprecationWorkflow.deprecationLog.messages[key] = |
| 286 | + ' { handler: "silence", ' + matchKey + ': ' + JSON.stringify(key) + ' }'; |
| 287 | + |
| 288 | + next(message, options); |
| 289 | +} |
| 290 | +``` |
| 291 | + |
| 292 | +</details> |
| 293 | + |
| 294 | +and at this point, we may as well build it into `ember` and not use an additional library at all, **and this is what the primary proposal of this RFC: build the deprecation workflow setup function in to ember**, so re-running through the setup steps: |
| 295 | + |
| 296 | +1. applications must have `@embroider/macros` installed by default. |
| 297 | +2. the app.js or app.ts can conditionally import a file which sets up the deprecation workflow |
| 298 | + ```diff app/app.js |
| 299 | + import Application from '@ember/application'; |
| 300 | + + import { importSync, isDevelopingApp, macroCondition } from '@embroider/macros'; |
| 301 | + |
| 302 | + import loadInitializers from 'ember-load-initializers'; |
| 303 | + import Resolver from 'ember-resolver'; |
| 304 | + import config from 'test-app/config/environment'; |
| 305 | + |
| 306 | + + if (macroCondition(isDevelopingApp())) { |
| 307 | + + importSync('<app-moduleName>/deprecation-workflow'); |
| 308 | + + } |
| 309 | + |
| 310 | + export default class App extends Application { |
| 311 | + modulePrefix = config.modulePrefix; |
| 312 | + podModulePrefix = config.podModulePrefix; |
| 313 | + Resolver = Resolver; |
| 314 | + } |
| 315 | + |
| 316 | + loadInitializers(App, config.modulePrefix); |
| 317 | + ``` |
| 318 | + this conditional import is now easily customizable for folks in their apps, so they could opt to _not_ strip deprecation messages in production, and see where deprecated code is being hit by users (reported via Sentry, BugSnag, or some other reporting tool) -- which may be handy for folks who have a less-than-perfect test suite (tests being the only current way to automatically detect where deprecated code lives). |
| 319 | +3. the `app/deprecation-workflow.js` would use the already public API, [`registerDeprecationHandler`](https://api.emberjs.com/ember/5.6/functions/@ember%2Fdebug/registerDeprecationHandler) |
| 320 | + ```js |
| 321 | + import { setupDeprecationWorkflow } from '@ember/debug'; |
| 322 | + |
| 323 | + setupDeprecationWorkflow({ |
| 324 | + throwOnUnhandled: true, |
| 325 | + handlers: [ |
| 326 | + /* ... handlers ... */ |
| 327 | + ] |
| 328 | + }); |
| 329 | + ``` |
| 330 | + |
| 331 | + |
| 332 | + |
| 333 | +## Unresolved questions |
| 334 | + |
| 335 | +n/a |
0 commit comments