ember-cli / ember-cli-deprecation-workflow

MIT License
165 stars 43 forks source link

Avoid the Ember global deprecation #118

Closed mixonic closed 3 years ago

mixonic commented 3 years ago

Following a suggestion at https://github.com/mixonic/ember-cli-deprecation-workflow/pull/107#issuecomment-849563611, use the self.require API to avoid using the Ember global when possible.

However there are other uses of deprecated Ember in this addon as well. These are found in the test suite. Make the test suite a bit more defensive and update deprecated API uses.

Summary:

TODO:

mixonic commented 3 years ago

This is included in https://github.com/mixonic/ember-cli-deprecation-workflow/releases/tag/v2.0.0-beta.5, please open issues if you have any trouble with it.

boris-petrov commented 3 years ago

@mixonic - thanks for trying to fix the deprecation warnings! However, this broke my app: Uncaught TypeError: require.has is not a function. The line is this one. Any ideas?

mixonic commented 3 years ago

@boris-petrov what version of Ember CLI, Ember, Node?

boris-petrov commented 3 years ago

Everything is the latest version - Ember CLI 3.27.0, Ember 3.27.5, Node 16.4.1.

mixonic commented 3 years ago

@boris-petrov Loader.js version? Are you using Embroider?

boris-petrov commented 3 years ago

As I said, everything is the latest version. :smile: Loader.js - 4.7.0. Not using Embroider, no.

Is require.has supposed to be there? require is but has is not...

mixonic commented 3 years ago

Yeah it is expected as part of the loader. I mean, clearly it is expected, right? It is tested here and I additionally booted an app to check these changes. I'll try to reproduce the issue with exactly the versions you mentioned. In the back of my head 'has' being missing sounds like something I have run into before, but I'll need to dive into the loader and some stuff.

Thanks for raising it, sounds like something we need to address quickly.

boris-petrov commented 3 years ago

Thank you for taking the time! Please tell me if you can't reproduce it - it might be something specific to my case (although I doubt that) - so in that case I'll try to figure out what's going on.

mixonic commented 3 years ago

@boris-petrov ok, I set an app to those versions and to the 2.0.0 release of workflow, and no luck reproducing.

has should be defined on window.require, I believe via this line in loader.js: https://github.com/ember-cli/loader.js/blob/master/lib/loader/loader.js#L332 (it indirectly sets it on requirejs but that variables refers to the same value (a function) as require.

mixonic commented 3 years ago

@boris-petrov if you drop a debugger onto the line where the exception is firing from, and add one inside loader.js, you might be able to rationalize what order things are happening in and if the loader is being set up as expected.

boris-petrov commented 3 years ago

@mixonic - thanks for trying it out. I should have debugged it before asking here. The problem is, as always, in ember-classic-decorator. I'll open an issue there. Thank you for the time and sorry for the confusion!