You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This PR changes how this addon works by loading the deprecation handling behavior in an application initializer (instead of from a vendor file). This allows us to import the registerDeprecationHandler functionality normally from @ember/debug, avoid the deprecation warning for accessing the Ember global.
I have removed the template deprecation stuff from index.js, but not 100% sure if/what that does 😬 It seems to be OK to me. Also not sure if the treeForLint stuff is still needed 🤷
I am using @embroider/macros to conditionally not include the registration code in production builds.
Note: This will not catch deprecations thrown very early in the build anymore (so before the app initializer runs). Not sure if there is a proper way to solve this without too much hackery 🤔 I noticed it with addons using Ember global in provided vendor JS files. Most of those uses are probably rather "hacky", but still the deprecations do get shown in the console instead of being completely silenced 🤔
Hmm. I pretty much hate initializers and would really like to avoid them if possible. A few possible ideas:
move our setup into the addon folder (like “normal”) and ask the app to import a setupDeprecationWorkflow() method in their side (e.g. app/app.js)
update the existing vendor code to use const registerDeprecationHandler = require.has(‘@ember/debug’) ? require(‘@ember/debug’).registerDeprecationHandler : Ember.Debug.registerDeprecationHandler
@rwjblue I think an explicit setup() makes sense. It is annoying, but would also make it easy to register your own handlers before or after. However, I'm not sure what it means for compile-time deprecations which I'm not sure use app.js?
@mydea I think this patch has effectively been landed in other steps, or addressed with other approaches. The final patch before we can cut a 2.0 is #118, IMO. Take a look.
I think this can be closed unless I'm missing something. Please let me know if I am! Thank you for your work here.
This file contains hidden or 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
2.02.0 blockers, see https://github.com/mixonic/ember-cli-deprecation-workflow/issues/109
3 participants
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.
This PR changes how this addon works by loading the deprecation handling behavior in an application initializer (instead of from a vendor file). This allows us to import the
registerDeprecationHandlerfunctionality normally from@ember/debug, avoid the deprecation warning for accessing theEmberglobal.I have removed the template deprecation stuff from index.js, but not 100% sure if/what that does 😬 It seems to be OK to me. Also not sure if the
treeForLintstuff is still needed 🤷I am using
@embroider/macrosto conditionally not include the registration code in production builds.This fixes #106
Also fixes #100