-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Use ESM entrypoint for require(ESM)
#16958
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
Conversation
commit: |
Should I add Markdown files in |
changelog_unreleased |
I think it's fine, it won't break |
@@ -15,6 +15,7 @@ | |||
"exports": { | |||
".": { | |||
"types": "./src/index.d.ts", | |||
"module-sync": "./src/index.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actual exports
exports: { |
Exactly I think. Old versions can't recognize |
Node 22.13, released today, removed the warning: |
Do you think we should also add |
Probably preferred |
Added a changelog reflecting the change in Node 22.13. |
This comment has been minimized.
This comment has been minimized.
I think you install require("prettier/index.cjs") === require("prettier")
require("prettier/index.mjs") === require("prettier") I don't think |
It works after $ npm init
This utility will walk you through creating a package.json file.
It only covers the most common items, and tries to guess sensible defaults.
See `npm help init` for definitive documentation on these fields
and exactly what they do.
Use `npm install <pkg>` afterwards to install a package and
save it as a dependency in the package.json file.
Press ^C at any time to quit.
package name: (prettier-test)
version: (1.0.0)
description:
entry point: (index.js)
test command:
git repository:
keywords:
author:
license: (ISC)
About to write to i:\prettier-test\package.json:
{
"name": "prettier-test",
"version": "1.0.0",
"description": "",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"author": "",
"license": "ISC"
}
Is this OK? (yes) yes
$ npm link ../prettier/dist
added 1 package, and audited 3 packages in 1s
1 package is looking for funding
run `npm fund` for details
found 0 vulnerabilities
$ node
Welcome to Node.js v22.13.0.
Type ".help" for more information.
> require("prettier").__esModule
true
> require("prettier") === require("prettier/index.mjs")
true
> require("prettier") === require("prettier/index.cjs")
false
> |
Node 20 can require(ESM) under a flag
Co-authored-by: fisker Cheung <lionkay@gmail.com>
require
if require(ESM) is enabledrequire(ESM)
Is this included in 3.5 or 4.0? |
Description
Pros:
index.cjs
will be skipped for Node 22.12 or later or bundlers supporting sync ESMs.require(ESM)
ed because it doesn't contain top-level awaits.Cons:
The following unpleasant warning will be out when we runOnly 22.12.xrequire("prettier")
:We may not be possible to accept this PR now due to this.If require(ESM) is stable in the future (Node 24?), we will be able to merge without hesitation.The warning is not shown since 22.13, so we don't have to care about this now.
Checklist
I’ve added tests to confirm my change works.(If changing the API or CLI) I’ve documented the changes I’ve made (in thedocs/
directory).changelog_unreleased/*/XXXX.md
file followingchangelog_unreleased/TEMPLATE.md
.✨Try the playground for this PR✨