Avoid using Ember global

ember-cli / ember-cli-deprecation-workflow

MIT License

165 stars 43 forks source link

Avoid using Ember global #107

Closed mydea closed 3 years ago

mydea commented 3 years ago

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 :grimacing: It seems to be OK to me. Also not sure if the treeForLint stuff is still needed :shrug:

I am using @embroider/macros to conditionally not include the registration code in production builds.

This fixes https://github.com/mixonic/ember-cli-deprecation-workflow/issues/106

Also fixes https://github.com/mixonic/ember-cli-deprecation-workflow/issues/100

mydea commented 3 years ago

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 :thinking: 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 :thinking:

rwjblue commented 3 years ago

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

mixonic commented 3 years ago

@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?

mixonic commented 3 years ago

@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 https://github.com/mixonic/ember-cli-deprecation-workflow/pull/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.

mydea commented 3 years ago

Great, looks good - I will try it out in our app soon. Thank you!