-
Notifications
You must be signed in to change notification settings - Fork 1k
Description
Currently this project does a lot of importing JSON files.
This is complicated to do in a cross environment way - some info in b21ac2e - tl;dr:
import matchGroupsJSON from '../config/matchGroups.json';- will "just work" in node if the
--experimental-json-modulesflag is enabled. - will get bundled properly by rollup, webpack, esbuild if this code ever ends up in a browser bundle.
Unfortunately starting with Node 17, an import assertion is required, so the syntax for importing JSON has changed to:
import matchGroupsJSON from '../config/matchGroups.json' assert {type: 'json'};@maxerickson tried adding these in f4a1170, but needed to revert in 7e08ce1 when the LTS versions of node in our CI build failed on this newer syntax.
See also: https://github.com/tc39/proposal-import-assertions
Unflagging the JSON module importer: nodejs/node#37375
Rejected Pull Request for a flag preserving the legacy syntax: nodejs/node#40210
It looks like esbuild, rollup, typescript etc are supporting the new syntax, or will soon:
evanw/esbuild#1871 (comment)
So this issue is mainly to keep an eye on things..
I don't think we can support Node 17 just yet, but we will eventually need to add these import assertions when we are targeting future node versions and can drop support for current node versions.
UPDATE 2/16/2022:
It surprised me to see that the import assertion requirement got backported to node 16.14. This means we really can't wait to implement this, as our contributors are already starting to run into parse errors 😠
Also - eslint is not supporting the syntax yet until it reaches stage 4, so I guess we will be disabling eslint for a while.
eslint/eslint#15305
So today I will:
- add this syntax anywhere we import a JSON file
- drop support for older versions of node in package.json and the CI action
- remove eslint from the test command for now 😢