-
Notifications
You must be signed in to change notification settings - Fork 895
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
Change import assertions to import attributes #9907
Conversation
Oh these things again.. This relates to #6014 |
Tried running build on this branch with v18/20/22 and it seems to be working
This shouldn't be a problem as package.json specifies that v18 or higher is required name-suggestion-index/package.json Lines 146 to 148 in 6b9b640
|
Yeah I think for now we might need to just stick to node 18 and 20. |
I've been using modified JS files with the suggested code changes in order to run the build and Wikidata scripts on Node 22. What I was/am doing was loading the modded files into my local clone of the repository, running the script, then discarding the changed JS files prior to uploading the script output. I knew uploading the modded files would break compatibility for anyone running older versions of Node, and I didn't want to unilaterally force others to upgrade. |
Not sure I understand what you mean here. This PR is compatible with all versions >= 18, including 18 and 20, so that means you think this is fine to merge? Or do you mean that it is a goal to avoid compatibility with v22? Edit: To be clear, my goal is not to introduce official support for v22. I just think it's nice that the project happens to build on v22, particularly when that can be achieved with relatively little effort, like is the case here. |
oh nice! I didn't realize that the nodejs/node#51622 In that case, then yes, we should just switch to the accepted syntax. Thanks for the PR! |
(re: #9907) Need new versions of 18, 20 to support `with` import attributes syntax
(re: osmlab#9907) Need new versions of 18, 20 to support `with` import attributes syntax
I couldn't get the project to build on Node v22, so I looked around and found this issue: https://stackoverflow.com/questions/78876691/syntaxerror-unexpected-identifier-assert-on-json-import-in-node-v22
After following one of the proposals, swapping import assertions to import attributes, the project builds on node v22 just fine. However, this means it won't build on Node v17 anymore.