module: fix --check on ambiguous ESM files - #65203
Open
bitpshr wants to merge 1 commit into
Open
Conversation
A `.js` file with no `"type"` in the nearest package.json has no format of its own, and `defaultGetFormat()` reports it as null. `--check` passed that null straight to `wrapSafe()`, which parses as CommonJS. Module syntax makes that parse bail out early, so the file was reported as valid and `--check` exited 0 even though it is not valid JavaScript under either goal. At load time the goal for such a file is decided by looking for module syntax in the source. Decide it the same way here, so the file is parsed as a module and its real syntax error is reported. Files whose format is known are unaffected, as are ambiguous files without module syntax, which are still parsed as CommonJS. Fixes: nodejs#65202 Signed-off-by: Paul Bouchon <mail@bitpshr.net>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #65203 +/- ##
==========================================
+ Coverage 90.15% 90.32% +0.16%
==========================================
Files 744 760 +16
Lines 242517 248538 +6021
Branches 45688 46893 +1205
==========================================
+ Hits 218642 224482 +5840
- Misses 15358 15460 +102
- Partials 8517 8596 +79
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #65202.
node --checkexits 0 on a.jsfile that contains module syntax and a genuine syntax error, when the file's format is ambiguous (no"type"in the nearest package.json).Such a file has no format of its own, and
defaultGetFormat()reports it asnull.checkSyntax()passed thatnullstraight towrapSafe(), which parses as CommonJS; the module syntax makes that parse bail out before the rest of the file is parsed, so nothing was reported.At load time the goal for an ambiguous file is decided by looking for module syntax in the source, so this decides it the same way before choosing how to parse.
Verified against a local build. Files whose format is known are unaffected, and ambiguous files without module syntax still parse as CommonJS, so sloppy-mode sources keep working:
.jsambiguous + module syntax + error.jsambiguous, sloppy-modewith.jsambiguous, plain syntax error.mjs, or"type"set.jsambiguous, valid ESMThe added fixture fails the existing
test-cli-syntax-badassertions before this change and passes after. Alltest-cli-syntax*suites pass locally.Fixes: #65202