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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 8 commits into from Jan 2, 2022
Merged

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

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
79a6c9a
doc: clarify `require` behavior with non `.js` extensions
aduh95 Dec 28, 2021
9d02904
Update doc/api/modules.md
aduh95 Dec 28, 2021
761197c
parse goal
aduh95 Dec 29, 2021
046e102
`.cjs` and `.mjs` in CJS
aduh95 Dec 30, 2021
5ed7e1c
`.cjs` files always need their extension to be included
aduh95 Jan 1, 2022
40b46cb
clean up links
aduh95 Jan 1, 2022
7639c2f
Remove enbling section as it doesn't have consensus
aduh95 Jan 2, 2022
9407cd5
remove repetition
aduh95 Jan 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions doc/api/modules.md
View file
Open in desktop
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