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

feat: support for CommonJS #156

Closed
wants to merge 4 commits into from
Closed

Conversation

wellwelwel
Copy link

@wellwelwel wellwelwel commented Nov 4, 2024

Hi, team 🙌

I would like to propose a compilation of index.js to make the project compatible with CommonJS, without changing the code or the supported versions (e.g, Node.js v18).

Consider it as wip.

Why?

From Node.js v23, an experimental feature (support for loading ES Module in require()) has been added, which brings up the following error for any command executed from npm:

(node:16132) ExperimentalWarning: CommonJS module <***>/lib/node_modules/npm/node_modules/debug/src/node.js is loading ES Module <***>/lib/node_modules/npm/node_modules/supports-color/index.js using require().
Support for loading ES Module in require() is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
  • This warning comes up with a simple npm ls, npm i or npm view npm version, for example.

The changes

The real changes were made to the package.json, where the new index.cjs file was automatically generated by the npm run build:cjs script (I used esbuild because it's dependency-free — it could be any compiler).

From a certain point of view, this PR could also be considered as a fix rather than a feature:

  • fix: support for Node.js v23 without experimental resources

Important

As there is no workflow that automates publishing, it's important to note that the compilation of index.cjs is done manually.

If we create a workflow to automate the publication, the index.cjs would become dispensable from the commit and could be compiled during the CI (which is my personal preference).

Note

I noticed that 4 tests were failing, but I didn't touch them or the main file (index.js).
It would be a pleasure to help with the correction of these tests if necessary.


Related

Some key issues, comments and PRs.


CC

!@sindresorhus, @Qix-, @ljharb, @mantoni (and all the community), it would be great if we could discuss this or, perhaps, an alternative approach 🤝

@LitoMore
Copy link
Contributor

LitoMore commented Nov 4, 2024

Better to ignore the .cjs file from Git. You can add the npm run build to prepublishOnly script.

And the .cjs distribution file needs to be tested.

@sindresorhus
Copy link
Member

No thanks. We don't plan to bring back CommonJS support.

@wellwelwel wellwelwel deleted the cjs-support branch November 4, 2024 08:33
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.

3 participants