-
-
Notifications
You must be signed in to change notification settings - Fork 407
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make deprecation workflow built in #1009
Merged
+335
−0
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
c0a338f
Make deprecation-workflow-built-in
NullVoxPopuli 2ef5ac2
Update
NullVoxPopuli 4304c23
Rename file
NullVoxPopuli c638e8b
answer why not this
NullVoxPopuli ffece8b
Forgat a :00
NullVoxPopuli d36dcef
Update text/1009-move-deprecation-workflow-to-apps.md
NullVoxPopuli 2e9c591
Update text/1009-move-deprecation-workflow-to-apps.md
NullVoxPopuli ad9a9bc
Update text/1009-move-deprecation-workflow-to-apps.md
NullVoxPopuli 759ee00
Update text/1009-move-deprecation-workflow-to-apps.md
NullVoxPopuli 45f675a
Update text/1009-move-deprecation-workflow-to-apps.md
NullVoxPopuli f920ec2
Update text/1009-move-deprecation-workflow-to-apps.md
NullVoxPopuli 9310137
Update text/1009-move-deprecation-workflow-to-apps.md
NullVoxPopuli 639b3dd
Merge branch 'master' into make-deprecation-workflow-built-in
wycats 41e4eed
Flip the suggestion to prefer using the addon in the blueprint
NullVoxPopuli 68f9b4d
Updates
NullVoxPopuli db2d4ff
Merge branch 'master' into make-deprecation-workflow-built-in
ef4 770a34d
Merge branch 'master' into make-deprecation-workflow-built-in
ef4 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,335 @@ | ||
--- | ||
stage: accepted | ||
start-date: 2024-02-22T00:00:00.000Z | ||
release-date: # In format YYYY-MM-DDT00:00:00.000Z | ||
release-versions: | ||
teams: # delete teams that aren't relevant | ||
- cli | ||
- data | ||
- framework | ||
- learning | ||
- typescript | ||
prs: | ||
accepted: https://github.com/emberjs/rfcs/pull/1009 | ||
project-link: | ||
suite: | ||
--- | ||
|
||
<!--- | ||
Directions for above: | ||
|
||
stage: Leave as is | ||
start-date: Fill in with today's date, 2032-12-01T00:00:00.000Z | ||
release-date: Leave as is | ||
release-versions: Leave as is | ||
teams: Include only the [team(s)](README.md#relevant-teams) for which this RFC applies | ||
prs: | ||
accepted: Fill this in with the URL for the Proposal RFC PR | ||
project-link: Leave as is | ||
suite: Leave as is | ||
--> | ||
|
||
# Move the deprecation workflow library to be installed in apps by default | ||
|
||
## Summary | ||
|
||
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. | ||
|
||
## Motivation | ||
|
||
Everyone needs a deprecation-workflow, and yet `ember-cli-deprecation-workflow` is not part of the default blueprint. | ||
|
||
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). | ||
|
||
|
||
## Detailed design | ||
|
||
|
||
Have `ember-cli-deprecation-workflow` installed by default. | ||
|
||
1. applications must have `@embroider/macros` installed by default. | ||
2. the app.js or app.ts can conditionally import a file which sets up the deprecation workflow | ||
```diff app/app.js | ||
import Application from '@ember/application'; | ||
+ import { importSync, isDevelopingApp, macroCondition } from '@embroider/macros'; | ||
|
||
import loadInitializers from 'ember-load-initializers'; | ||
import Resolver from 'ember-resolver'; | ||
import config from 'test-app/config/environment'; | ||
|
||
+ if (macroCondition(isDevelopingApp())) { | ||
+ importSync('./deprecation-workflow'); | ||
+ } | ||
|
||
export default class App extends Application { | ||
modulePrefix = config.modulePrefix; | ||
podModulePrefix = config.podModulePrefix; | ||
Resolver = Resolver; | ||
} | ||
|
||
loadInitializers(App, config.modulePrefix); | ||
``` | ||
3. then in `app/deprecation-workflow.js` would use the already public API, | ||
```js | ||
import setupDeprecationWorkflow from 'ember-cli-deprecation-workflow'; | ||
|
||
setupDeprecationWorkflow({ | ||
/** | ||
false by default, but if a developer / team wants to be more aggressive about being proactive with | ||
handling their deprecations, this should be set to "true" | ||
*/ | ||
throwOnUnhandled: false, | ||
handlers: [ | ||
/* ... handlers ... */ | ||
] | ||
}); | ||
``` | ||
|
||
|
||
This follows the README of [ember-cli-deprecation-workflow](https://github.com/ember-cli/ember-cli-deprecation-workflow?tab=readme-ov-file#getting-started). | ||
|
||
|
||
|
||
## How we teach this | ||
|
||
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. | ||
|
||
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`. | ||
|
||
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. | ||
|
||
## Drawbacks | ||
|
||
For older projects, this could be _a_ migration. But as it is additional blueprint boilerplate, it is optional. | ||
|
||
## Alternatives | ||
|
||
There are only a few features of `ember-cli-deprecation-workflow` that we need to worry about: | ||
- enabled or not - do we check deprecations at all, or ignore everything (current default) | ||
- `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. | ||
|
||
- `window.flushDeprecations()` - prints the list of deprecations encountered since the last page refresh | ||
- 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) | ||
- Logging / Ignoring / Throwing - when encountering a matched deprecation (whether by id or by regex, how should it be handled?) | ||
|
||
|
||
However, folks can get a basic deprecation-handling workflow going in their apps without the above features, | ||
|
||
1. applications must have `@embroider/macros` installed by default. | ||
2. the app.js or app.ts can conditionally import a file which sets up the deprecation workflow | ||
```diff app/app.js | ||
import Application from '@ember/application'; | ||
+ import { importSync, isDevelopingApp, macroCondition } from '@embroider/macros'; | ||
|
||
import loadInitializers from 'ember-load-initializers'; | ||
import Resolver from 'ember-resolver'; | ||
import config from 'test-app/config/environment'; | ||
|
||
+ if (macroCondition(isDevelopingApp())) { | ||
+ importSync('./deprecation-workflow'); | ||
+ } | ||
|
||
export default class App extends Application { | ||
modulePrefix = config.modulePrefix; | ||
podModulePrefix = config.podModulePrefix; | ||
Resolver = Resolver; | ||
} | ||
|
||
loadInitializers(App, config.modulePrefix); | ||
``` | ||
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). | ||
3. the `app/deprecation-workflow.js` would use the already public API, [`registerDeprecationHandler`](https://api.emberjs.com/ember/5.6/functions/@ember%2Fdebug/registerDeprecationHandler) | ||
```ts | ||
import { registerDeprecationHandler } from '@ember/debug'; | ||
|
||
import config from '<app-moduleName>/config/environment'; | ||
|
||
const SHOULD_THROW = config.environment !== 'production'; | ||
const SILENCED_DEPRECATIONS: string[] = [ | ||
// Add ids of deprecations you temporarily want to silence here. | ||
]; | ||
|
||
registerDeprecationHandler((message, options, next) => { | ||
if (!options) { | ||
console.error('Missing options'); | ||
throw new Error(message); | ||
} | ||
|
||
if (SILENCED_DEPRECATIONS.includes(options.id)) { | ||
return; | ||
} else if (SHOULD_THROW) { | ||
throw new Error(message); | ||
} | ||
|
||
next(message, options); | ||
}); | ||
``` | ||
|
||
|
||
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. | ||
|
||
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: | ||
|
||
<details><summary>ember-cli-deprecation-workflow/index.js</summary> | ||
|
||
```js | ||
import { registerDeprecationHandler } from '@ember/debug'; | ||
|
||
const LOG_LIMIT = 100; | ||
|
||
export default function setupDeprecationWorkflow(config) { | ||
self.deprecationWorkflow = self.deprecationWorkflow || {}; | ||
self.deprecationWorkflow.deprecationLog = { | ||
messages: {}, | ||
}; | ||
|
||
registerDeprecationHandler((message, options, next) => | ||
handleDeprecationWorkflow(config, message, options, next), | ||
); | ||
|
||
registerDeprecationHandler(deprecationCollector); | ||
|
||
self.deprecationWorkflow.flushDeprecations = flushDeprecations; | ||
} | ||
|
||
let preamble = `import setupDeprecationWorkflow from 'ember-cli-deprecation-workflow'; | ||
|
||
setupDeprecationWorkflow({ | ||
workflow: [ | ||
`; | ||
|
||
let postamble = ` ] | ||
});`; | ||
|
||
export function detectWorkflow(config, message, options) { | ||
if (!config || !config.workflow) { | ||
return; | ||
} | ||
|
||
let i, workflow, matcher, idMatcher; | ||
for (i = 0; i < config.workflow.length; i++) { | ||
workflow = config.workflow[i]; | ||
matcher = workflow.matchMessage; | ||
idMatcher = workflow.matchId; | ||
|
||
if (typeof idMatcher === 'string' && options && idMatcher === options.id) { | ||
return workflow; | ||
} else if (typeof matcher === 'string' && matcher === message) { | ||
return workflow; | ||
} else if (matcher instanceof RegExp && matcher.exec(message)) { | ||
return workflow; | ||
} | ||
} | ||
} | ||
|
||
export function flushDeprecations() { | ||
let messages = self.deprecationWorkflow.deprecationLog.messages; | ||
let logs = []; | ||
|
||
for (let message in messages) { | ||
logs.push(messages[message]); | ||
} | ||
|
||
let deprecations = logs.join(',\n') + '\n'; | ||
|
||
return preamble + deprecations + postamble; | ||
} | ||
|
||
export function handleDeprecationWorkflow(config, message, options, next) { | ||
let matchingWorkflow = detectWorkflow(config, message, options); | ||
if (!matchingWorkflow) { | ||
if (config && config.throwOnUnhandled) { | ||
throw new Error(message); | ||
} else { | ||
next(message, options); | ||
} | ||
} else { | ||
switch (matchingWorkflow.handler) { | ||
case 'silence': | ||
// no-op | ||
break; | ||
case 'log': { | ||
let key = (options && options.id) || message; | ||
|
||
if (!self.deprecationWorkflow.logCounts) { | ||
self.deprecationWorkflow.logCounts = {}; | ||
} | ||
|
||
let count = self.deprecationWorkflow.logCounts[key] || 0; | ||
self.deprecationWorkflow.logCounts[key] = ++count; | ||
|
||
if (count <= LOG_LIMIT) { | ||
console.warn('DEPRECATION: ' + message); | ||
if (count === LOG_LIMIT) { | ||
console.warn( | ||
'To avoid console overflow, this deprecation will not be logged any more in this run.', | ||
); | ||
} | ||
} | ||
|
||
break; | ||
} | ||
case 'throw': | ||
throw new Error(message); | ||
default: | ||
next(message, options); | ||
break; | ||
} | ||
} | ||
} | ||
|
||
export function deprecationCollector(message, options, next) { | ||
let key = (options && options.id) || message; | ||
let matchKey = options && key === options.id ? 'matchId' : 'matchMessage'; | ||
|
||
self.deprecationWorkflow.deprecationLog.messages[key] = | ||
' { handler: "silence", ' + matchKey + ': ' + JSON.stringify(key) + ' }'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whether in deprecation-workflow or in a new implementation, I think we should also capture |
||
|
||
next(message, options); | ||
} | ||
``` | ||
|
||
</details> | ||
|
||
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: | ||
|
||
1. applications must have `@embroider/macros` installed by default. | ||
2. the app.js or app.ts can conditionally import a file which sets up the deprecation workflow | ||
```diff app/app.js | ||
import Application from '@ember/application'; | ||
+ import { importSync, isDevelopingApp, macroCondition } from '@embroider/macros'; | ||
|
||
import loadInitializers from 'ember-load-initializers'; | ||
import Resolver from 'ember-resolver'; | ||
import config from 'test-app/config/environment'; | ||
|
||
+ if (macroCondition(isDevelopingApp())) { | ||
+ importSync('<app-moduleName>/deprecation-workflow'); | ||
+ } | ||
|
||
export default class App extends Application { | ||
modulePrefix = config.modulePrefix; | ||
podModulePrefix = config.podModulePrefix; | ||
Resolver = Resolver; | ||
} | ||
|
||
loadInitializers(App, config.modulePrefix); | ||
``` | ||
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). | ||
3. the `app/deprecation-workflow.js` would use the already public API, [`registerDeprecationHandler`](https://api.emberjs.com/ember/5.6/functions/@ember%2Fdebug/registerDeprecationHandler) | ||
```js | ||
import { setupDeprecationWorkflow } from '@ember/debug'; | ||
|
||
setupDeprecationWorkflow({ | ||
throwOnUnhandled: true, | ||
handlers: [ | ||
/* ... handlers ... */ | ||
] | ||
}); | ||
``` | ||
|
||
|
||
|
||
## Unresolved questions | ||
|
||
n/a |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deprecate
calls from ember in production are stripped as part of Ember's build, not the project's, so there would be no way to keep them in production, right now