Skip to content
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

Open
8 tasks
shaunanoordin opened this issue Aug 28, 2023 · 3 comments
Open
8 tasks

Change to ESM (ECMAScript module) #165

shaunanoordin opened this issue Aug 28, 2023 · 3 comments

Comments

@shaunanoordin
Copy link
Member

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.

// package.json
{
  "type": "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.json

Step 2: jest.config.js

  • Change module.exports = config to export default config
  • Check that it works: run npm run test

Step 3: webpack.dev.js and webpack.prod.js

  • Change module.exports = { ... } to const config = { ... } plus export default config
  • Change const whatever = require('whatever') to import whatever from 'whatever'
  • Add missing __dirname variable
  • ❗ Find replacement for require.resolve()
  • Check that it works pt 1: run npm start and browse local.zooniverse.org:8080
  • Check that it works pt 2: npm 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 of export ..., in PR 163's discussions

@eatyourgreens
Copy link
Contributor

Where you're currently doing something like:

import { subjects } from '@zooniverse/panoptes-js'

you're importing the entire client object, then destructuring client.subjects. That means that you're loading in dependencies that you don't need, like jose.

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'

@eatyourgreens
Copy link
Contributor

Node's documentation on interoperability with CommonJS, including __dirname and require.resolve.

https://nodejs.org/api/esm.html#differences-between-es-modules-and-commonjs

Also worth noting that you can rename the webpack config to .cjs if you need to use require.

@eatyourgreens
Copy link
Contributor

If you're building a new package from scratch, then Rollup might be a better choice than Webpack, for bundling an ES module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants