Skip to content
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
Prev Previous commit
Next Next commit
Update doc/api/modules.md
Co-authored-by: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
aduh95 and targos authored Dec 28, 2021
commit 9d02904215b334c2138301ecc3eff1f8723fd080
2 changes: 1 addition & 1 deletion doc/api/modules.md
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ required filename with the added extensions: `.js`, `.json`, and finally
`.node`.

`.json` files are parsed as JSON text files, `.node` files are interpreted as
compiled addon modules loaded with `process.dlopen()`. Files using an unknown
compiled addon modules loaded with `process.dlopen()`. Files using any other
extension (or no extension at all) are parsed as JavaScript text files.
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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


Comment thread
aduh95 marked this conversation as resolved.
A required module prefixed with `'/'` is an absolute path to the file. For
Expand Down