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

doc: clarify require behavior with non .js extensions #41345

Merged
merged 8 commits into from
Jan 2, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions doc/api/modules.md
Original file line number Diff line number Diff line change
Expand Up @@ -377,9 +377,9 @@ If the exact filename is not found, then Node.js will attempt to load the
required filename with the added extensions: `.js`, `.json`, and finally
`.node`.

`.js` files are interpreted as JavaScript text files, and `.json` files are
parsed as JSON text files. `.node` files are interpreted as compiled addon
modules loaded with `process.dlopen()`.
`.json` files are parsed as JSON text files, `.node` files are interpreted as
compiled addon modules loaded with `process.dlopen()`. Files using an unknown
extension (or no extension at all) are parsed as JavaScript text files.
Copy link
Member

Choose a reason for hiding this comment

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

“JavaScript text files” doesn’t tell me whether it’s as a Script/CJS, or as ESM, since JavaScript text files have two parse goals. (i know the answer - that it’s CJS - but it’d be good if this sentence was unambiguous)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the wording here is carefully chosen so it applies to both CJS and ESM. Any non-.json-nor-.node extension is treated as JS text file, which may be either a module (in which case the require call will throw) or a script. I'm not a fan of ambiguity myself, but I don't think here is the place to list the rules to determine if a JS text file is parsed as script or module. wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

I think if ambiguity is the intention, your PR as-is indeed captures the ambiguity correctly :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

The CommonJS loader does parse JS as CommonJS scripts always though, so I think @ljharb's seeking a clarification does make sense - there isn't ambiguity in play.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Calling require on an ES module doesn't make it CJS, it throws a ERR_REQUIRE_ESM. Replacing JavaScript text files with CommonJS modules would not be an improvement imho. Do you have a suggestion that removes the ambiguity?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes you're right, .mjs and .js in "type": "module" are excluded by this error, which does form an ambiguity of sorts.

Perhaps then explain exactly that, and that will solidify the ESM distinction:

.mjs or .js files within a "type": "module" package boundary throw an ERR_REQUIRE_ESM error. All other files are parsed as CommonJS scripts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What if we link to the ad-hoc section in packages.md?

Copy link
Member

Choose a reason for hiding this comment

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

Except for json and .node and wasm files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

require('./file.wasm') loads file.wasm as CJS:

mkdir repro
cd repro
echo 'console.log(this !== undefined)' > file.wasm
echo 'require("./file.wasm")' | node

aduh95 marked this conversation as resolved.
Show resolved Hide resolved

aduh95 marked this conversation as resolved.
Show resolved Hide resolved
A required module prefixed with `'/'` is an absolute path to the file. For
example, `require('/home/marco/foo.js')` will load the file at
Expand Down