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

chore(cjs/ems): Re-add CJS for dual module support #1234

Closed
wants to merge 2 commits into from

Conversation

loynoir
Copy link

@loynoir loynoir commented Aug 5, 2021

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • New feature
  • Other, please explain:

What changes did you make? (provide an overview)
Re-add CJS support which dropped in v3.0.0-beta.10.
But ESM & CJS dual module support exists <= v3.0.0-beta.9.

Which issue (if any) does this pull request address?
#1226
#1230

Is there anything you'd like reviewers to know?
ESM & CJS dual module support exists in v3.0.0-beta.9.
As I'm a big fans of ESM, I understand CJS is dropped.
But I don't think it's time to drop CJS support.

PS: As may not re-add CJS, I use esbuild for easy mainteinance.

"exports": {

"build": "rollup -c",

format: 'cjs',

@jimmywarting
Copy link
Collaborator

This is essentially a duplicate variant of PR #1227

@loynoir
Copy link
Author

loynoir commented Aug 5, 2021

@jimmywarting
Sorry, I did not see that pull, as jest fail, I try to fix that ASAP.
And please consider re-add cjs support, although I'm a big fan of ESM, but that whole ecosystem haven't reached there yet.
So, I think might be better to support both nowadays.

@LinusU
Copy link
Member

LinusU commented Aug 5, 2021

@loynoir did you read the discussion in #1227? e.g. #1227 (comment)

Feel free to lay out arguments and continue the discussion there ☺️

@jimmywarting
Copy link
Collaborator

Locking b/c it's nicer to keep the discussion in one place.
(We still keep a open mind and looking for a solution - it dose not mean we will overlook this PR)

Feel free to keep pushing commits and move the discussion to #1227 where most of the discussion is already happening

@node-fetch node-fetch locked and limited conversation to collaborators Aug 13, 2021
@jimmywarting jimmywarting changed the title Re-add ESM & CJS dual module support chore(cjs/ems) Re-add CJS for dual module support Aug 21, 2021
@jimmywarting jimmywarting changed the title chore(cjs/ems) Re-add CJS for dual module support chore(cjs/ems): Re-add CJS for dual module support Aug 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants