-
Notifications
You must be signed in to change notification settings - Fork 0
[DEV] Upgrade to Ember 6.1 #29
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
base: master
Are you sure you want to change the base?
Conversation
eacfb37 to
44dfefe
Compare
|
|
||
| // fetch is built into Node.js 20, so at some stage we should consider | ||
| // removing this dependency. | ||
| // eslint-disable-next-line no-redeclare |
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.
@antonivanopoulos @andybluntish I'm a little stuck on making a decision on these two imports. They are slightly different in nature.
For crypto, I believe what eslint is trying to tell me is that I can just call crypto without importing it on the version of node that I'm on, however I can't find supporting docs for that, most examples online continue to import like this.
For fetch, I believe eslint is aware that node ships with a fetch module, however this is using the installed node-fetch package, hence the namespace clash.
The thing that is really stopping me from making decisions is that there is no way to test this out, apart from pointing one of our apps at it and seeing if it works. While that would be effective, I kinda want to be more certain, especially as we are using private keys to sign things.
I'd like to write some tests but exporting a file from the lib dir into a test for an addon isn't straightforward. (lol at the supporting documentation for testing addons https://cli.emberjs.com/release/writing-addons/intro-tutorial/#testinganaddon).
Any thoughts would be valued.
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.
The issue here is going to be the eslint setup. We're setting up the globals config to include globals.browser in all js files (**/*.{js,gjs}).
Later on we're including lib/**/*.js in the "node files" config, which adds the node globals, but this isn't overriding the previous config—is additive.
So eslint thinks that this file has access to both the browser and nodejs globals, and in this case is complaining that we're importing crypto because that exists in globals.browser.
For this plugin, we can probably just remove all the browser-specific eslint config, since there isn't any code that will run there.
Uh oh!
There was an error while loading. Please reload this page.