-
Notifications
You must be signed in to change notification settings - Fork 0
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 to ESM (ECMAScript module) #165
Comments
Where you're currently doing something like: import { subjects } from '@zooniverse/panoptes-js' you're importing the entire client object, then destructuring You should be able to get around this by importing just the object that you need. Something like this: import subjects from '@zooniverse/panoptes-js/resources/subjects' |
Node's documentation on interoperability with CommonJS, including https://nodejs.org/api/esm.html#differences-between-es-modules-and-commonjs Also worth noting that you can rename the webpack config to |
If you're building a new package from scratch, then Rollup might be a better choice than Webpack, for bundling an ES module. |
Project Improvement / Code Standardisation
Suggestion as of f797e52
We should change this project to an ESM format. This can be done by adding
{ "type": "module" }
to package.json (docs), which tells Node.js to treat our project as an ECMAScript module, instead of the default CommonJS module.Q: is it essential?
A: nope!
Q: why do this?
A: code standardisation.
Q: what's the catch?
A: it kinda breaks the current builds.
Things To Do
Step 1: Add
{ "type": "module" }
to package.jsonStep 2:
jest.config.js
module.exports = config
toexport default config
npm run test
Step 3:
webpack.dev.js
andwebpack.prod.js
module.exports = { ... }
toconst config = { ... }
plusexport default config
const whatever = require('whatever')
toimport whatever from 'whatever'
__dirname
variable__dirname
into CommonJS modules but not ESM (??), so we'll need to set up that var somehow.import.meta
)require.resolve()
import.meta.resolve
, but as of Node 20, this is experimental as hecknpm start
and browse local.zooniverse.org:8080npm run build
Status
Low priority for the moment, but if we can solve those gotchas, it'd be a nice improvement to have to the code. Thanks to Jim for pointing out why running Jest requires
module.exports = ...
in the config instead ofexport ...
, in PR 163's discussionsThe text was updated successfully, but these errors were encountered: