-
Notifications
You must be signed in to change notification settings - Fork 188
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
Adopt type=module #202
Adopt type=module #202
Conversation
follow changes in d3-format: * type=module * add exports * remove zip * license: ISC * update dependencies TODO: the tests on *global* fail.
Do the tests on global fail because the code is now in strict mode, so global is undefined? Or is it globalThis? |
From what I see global is OK (when the tests run), but this is undefined. And eslint complains that global is undef. |
I think since the code is now in strict mode we expect this to be undefined. |
another (last?) issue, is how to load the barley test data (json file). what I do here gives an eslint error |
Same thing with |
"ecmaVersion": 8 | ||
}, | ||
"env": { | ||
"es6": true, |
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.
This needs to be at least version 11 for the import.meta.url
to work.
I think we need to decide if we want to go with es6 or jump all the way up to es2020 (which version 11 is part of). It is possible to keep env es6
and use ecmaVersion 11
, but not sure if that is the way to go here. (I'd rather just jump all the way up to es2020, or es2021)
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.
I just need to find a way to load the json file that makes everyone happy (ie tests and eslint)
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.
Yes, let’s not use require here. Let’s use fs.readFileSync or similar.
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.
You can also rename the file to a .js
instead of .json
, and inside of the js-file you can do a default export [...]
. This way its possible to import the js-file into the tests and they will run as usual. Then you wont need fs.readFileSync.
Not sure what your usual coding practices have been with d3js, so I'll let you guys decide whats best :)
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.
now let's see if it passes CI :)
follow changes in d3-format:
TODO: