Skip to content

docs: Add example for loading ESM from CommonJS#1236

Merged
jimmywarting merged 14 commits intomainfrom
features/load-from-docs
Aug 28, 2021
Merged

docs: Add example for loading ESM from CommonJS#1236
jimmywarting merged 14 commits intomainfrom
features/load-from-docs

Conversation

@jimmywarting
Copy link
Collaborator

@jimmywarting jimmywarting commented Aug 6, 2021

Purpose

Few ppl started complaining about beta-10 going ESM and are unable to load it. The purpose of this PR is to provide them a alternative method to load ESM modules from CJS

Changes

Have only updated some documents and suggested this way of loading fetch:

// mod.cjs
const fetch = (...args) => import('node-fetch').then(mod => mod.default(...args));

Additional information

There are both pro & cons with ESM.
Bundle all dependencies and having own instances is not desirable. It can make working with different whatwg:streams versions incompatible


  • I prefixed the PR-title with docs:
  • I updated ./docs/CHANGELOG.md with a link to this PR or Issue
  • I updated ./docs/v3-UPGRADE-GUIDE
  • I updated the README.md
  • Should maybe link to a ESM upgrade guide blog/article of how to upgrade a cjs package to esm and how to use different compilers? ...any suggestions? can do this later...

Copy link
Member

@LinusU LinusU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat 👍

@jimmywarting jimmywarting added this to the Version 3.0.0 milestone Aug 12, 2021
@jimmywarting jimmywarting requested a review from gr2m August 13, 2021 12:36
@jimmywarting
Copy link
Collaborator Author

jimmywarting commented Aug 16, 2021

What to you think about adding this bootstrap approach as a way of loading a ESM package from a complete legacy commonjs codebase?

// bootstrap.js
import('node-fetch').then(({
  default: fetch,
  Headers,
  Request,
  Response
}) => {
  globalThis.fetch = fetch
  globalThis.Headers = Headers
  globalThis.Request = Request
  globalThis.Response = Response
  require('./app.js')
})

Shorter version:

// bootstrap.js
import('node-fetch').then(({
  default: fetch,
  ...rest
}) => {
  Object.assign(globalThis, rest, { fetch })
  require('./app.js')
})

This had me thinking... can you use preload?

-r, --require module#

Preload the specified module at startup.

@Richienb
Copy link
Member

If you're going to assign globals within an asynchronously executed function, then you might as well just await it and get the values that way:

const {
  default: fetch,
  Headers,
  Request,
  Response
} = await import("node-fetch")

This would be best executed within asynchronous functions since CommonJS does not support top-level await.

@jimmywarting
Copy link
Collaborator Author

jimmywarting commented Aug 21, 2021

I where thinking about linking to https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c but it seems highly debatable about good and bad things and includes a few negative comments.
It's a very good guide. I almost wish it was locked and had all some comments removed... i wish it was targeted more at a general scale and didn't had so many comments about Typescript, React vs Javascript comments

There is also a few forks worth mentioning: https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c/forks

We also have: https://blog.sindresorhus.com/get-ready-for-esm-aa53530b3f77 and https://blog.sindresorhus.com/hello-modules-d1010b4e777b

Copy link
Member

@LinusU LinusU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked thru everything again, looks great 👍

@Richienb Richienb changed the title docs: Example loading ESM from cjs docs: Add example for loading ESM from CommonJS Aug 21, 2021
@jimmywarting jimmywarting requested a review from Richienb August 21, 2021 17:24
@jimmywarting
Copy link
Collaborator Author

@Richienb i fixed all your suggestions, please have another look

@ThaUnknown
Copy link

ThaUnknown commented Aug 27, 2021

is 40 lines of documentation what's preventing us from making v3 stable?

@jimmywarting
Copy link
Collaborator Author

jimmywarting commented Aug 27, 2021

is 40 lines of documentation what's preventing us from making v3 stable?

kind of... this is the only thing left in the v3 milestone
ppl have had concerns/problem importing pure ESM package in beta-10. This address those problem + the docs where a bit outdated

Only need 1 more acceptances from one of the maintainers and then we can release/tag v3 as stable


...Feels like some of the maintainers don't have much time or interest anymore that's why updates comes so rarely
Should maybe invite new maintainers

@jimmywarting jimmywarting merged commit 2f1b426 into main Aug 28, 2021
@jimmywarting jimmywarting deleted the features/load-from-docs branch August 28, 2021 00:10
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

Successfully merging this pull request may close these issues.

v3 upgrade Docs needs to be updated

5 participants