Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Avoid using Ember global #107

Closed
wants to merge 1 commit into from
Closed

Avoid using Ember global #107

wants to merge 1 commit into from

Conversation

mydea
Copy link

@mydea mydea commented May 25, 2021 •
edited
Loading

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.

This fixes #106

Also fixes #100

@mydea mydea force-pushed the fn/avoid-ember-global branch from 60d4de0 to 7818117 Compare May 25, 2021 09:29
@mydea
Copy link
Author

mydea commented May 25, 2021

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 🤔

@mixonic mixonic mentioned this pull request May 27, 2021
Closed
5 tasks
@mixonic mixonic added the 2.0 2.0 blockers, see https://github.com/mixonic/ember-cli-deprecation-workflow/issues/109 label May 27, 2021
@rwjblue
Copy link
Member

rwjblue commented May 27, 2021

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
Copy link
Member

mixonic commented May 27, 2021

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

This was referenced May 28, 2021
Closed
Merged
@mixonic
Copy link
Member

mixonic commented Jul 3, 2021

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

@mydea
Copy link
Author

mydea commented Jul 5, 2021

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

@mydea mydea closed this Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
2.0 2.0 blockers, see https://github.com/mixonic/ember-cli-deprecation-workflow/issues/109
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

deprecationWorkflow does not exist? Deprecation warnings with Ember 3.27
3 participants
@mydea @rwjblue @mixonic