Skip to content

[25.x] module: fix extensionless CJS files in type:module packages#62083

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

Merged
aduh95 merged 1 commit into from
Mar 10, 2026
Merged

[25.x] module: fix extensionless CJS files in type:module packages #62083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions lib/internal/modules/cjs/loader.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1952,12 +1952,12 @@ Module._extensions['.js'] = function(module, filename) {
format = 'typescript';
}
} else if (path.extname(filename) === '') {

@28vsp2qx6n-collab28vsp2qx6n-collabMar 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  • [ ]

// Extensionless files skip the .js suffix check above. When type is explicit,
// follow it so ESM syntax surfaces as SyntaxError for commonjs instead of
// silently delegating to ESM.
// Extensionless files skip the .js suffix check above. When type is commonjs, follow it so ESM
// syntax surfaces as SyntaxError. For type: module, leave format undefined so our syntax
// detection handles it (allowing CJS extensionless files in ESM packages).
Comment on lines +1956 to +1957

@GeoffreyBoothGeoffreyBoothMar 9, 2026

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.

Does our syntax detection handle it? I think to confirm that it does, this PR also needs a test that requiring an extensionless file with ESM syntax in a type: module package scope also works.

This is also the part that worries me, though: according to our docs, syntax detection happens for

Files with a .js extension or no extension; and either no controlling package.json file or one that lacks a type field.

Assuming the comment here does what it claims to do, then this PR is creating a new exception to our docs: syntax detection applies in the above case, and for the string input, and for extensionless files in a type: module context. I thought we wanted syntax detection to never apply when a type field was set; that's what the warning we print tells users to add to avoid the performance hit of syntax detection. That's why this PR in general feels like the wrong solution to me.

Alternatively if this comment is just wrong and there's no syntax detection involved here, then we should fix this comment (and the PR description).

@joyeecheungjoyeecheungMar 9, 2026•
edited
Loading

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.

this PR is creating a new exception to our docs: syntax detection applies in the above case, and for the string input, and for extensionless files in a type: module context.

I think this needs to factor in that it's not a new exception, it's still the status quo for v20-v24. #61600 removed the exceptions on v25, and this PR is only partially putting a small part of the exception back to prevent regressions on v25, since that part is documented (but not the others). In v26 the exception will be fully disallowed since it'll have #61600 in full.

Alternatively, if we want to forbid syntax detection entirely, to make it strictly follow the documented exception it should change to if (typeFromPjson === 'module') format = 'commonjs', but that feels even more wrong and it can only happen on v25....syntax detection is mostly just a mechanics to relax the interpretation of the type field to be less wrong, instead of completely going the opposite of what type field says (which is what got documented) and forbid extensionless ESM when there's type: module (meanwhile, without type in package.json it still allows extensionless ESM since 20).

@GeoffreyBoothGeoffreyBoothMar 9, 2026

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.

If we're currently doing any syntax detection for files where the controlling package.json file has a type field, that's a bug. That's not documented in the syntax detection section, nor in https://nodejs.org/docs/v25.7.0/api/modules.html#enabling. The type field is supposed to prevent syntax detection, full stop, for all files including extensionless ones. That's why we print this:

node/lib/internal/modules/esm/get_format.js

Lines 148 to 150 in 17c76c1

constwarning=`Module type of ${url} is not specified and it doesn't parse as CommonJS.\n`+
'Reparsing as ES module because module syntax was detected. This incurs a performance overhead.\n'+
`To eliminate this warning, add "type": "module" to ${pjsonPath}.`;

So I'm not sure what exception you're referring to? Are the docs and this warning all wrong and we've been secretly doing syntax detection for extensionless files in type: module scopes prior to #61600?

@joyeecheungjoyeecheungMar 10, 2026•
edited
Loading

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.

Are the docs and this warning all wrong and we've been secretly doing syntax detection for extensionless files in type: module scopes prior to #61600?

Yes, or more precisely, we had been ignoring the type field for required extensionless files before 61600 - and still do on v20-v24. We didn't even read the package.json's type field at all for required extensionless files until 61600. That's why I say reverting it is going to make it more wrong.

As it turned out, if the type field is module, treating the required extensionless file as ESM caused a regression from the documented exception, so we have two choices to fix it for 25.

  1. If the type field is module, treat the required file as commonjs (this sounds even weirder, but it's what the doc says, and what yargs 17 expects). Note that this could still be breaking because it then disallows required extensionless ESM even with type: module, which has been available on v20-24.
  2. If type field is module, ignore the type field (what this PR does), so required extensionless files under this package.json can be either commonjs (what yargs 17 needs) or ESM (less weird for other cases, and what actually respecting the type field would've allowed), which is also the behavior we have for v20-v24.

Note that this is unrelated to entry points, and only for required extensionless files.

@GeoffreyBoothGeoffreyBoothMar 10, 2026

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.

These exceptions are nonsensical. I don't think the solution is to update our docs to correspond with these buggy code paths. We should just fix the code.

If a type field is set, no syntax detection should happen, period. No exceptions around extensionless files or require vs import vs entry point.

@joyeecheungjoyeecheungMar 11, 2026•
edited
Loading

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.

This now feels like talking in circles...

I don't think the solution is to update our docs to correspond with these buggy code paths

I don't think anyone is proposing that. The proposal is to just remove the exception in docs in 26, which is being done in #62176

If a type field is set, no syntax detection should happen, period. No exceptions around extensionless files or require vs import vs entry point.

I think that was agreed all along, and it was what 61600 implemented, and we already tried to ship it in 25, and it turned out there was this regression, and that's how we realized there was a documented, conflicting exception elsewhere. So for 25 the exception for type: module + extensionless commonjs needs to remain in the docs with the code aligned back due to the regression - which was done in this PR. We are still removing other undocumented exceptions (notice how this PR still leaves type: commonjs properly respected, which was added in 61600, since there isn't a documented exception for it). But we will keep 61600 for 26 to disallow exceptions.

pkg = packageJsonReader.getNearestParentPackageJSON(filename);
const typeFromPjson = pkg?.data?.type;
if (typeFromPjson === 'commonjs' || typeFromPjson === 'module') {
if (typeFromPjson === 'commonjs') {
format = typeFromPjson;
}
Comment on lines +1960 to 1962

@guybedfordguybedfordMar 4, 2026

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.

Would it be possible to have extensionless files entirely ignore the package.json "type"? That seems like a more pragmatic approach given that they are designed to exist outside of package boundaries in most "binary execution" use cases.

}
Expand Down
10 changes: 10 additions & 0 deletions test/es-module/test-extensionless-esm-type-commonjs.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,3 +17,13 @@ spawnSyncAndAssert(process.execPath, [
stdout: /script STARTED[\s\S]*v\d+\./,
trim: true,
});

// CJS extensionless file inside a type: "module" package should work
// when require()'d. Regression test for https://github.com/nodejs/node/issues/61971
spawnSyncAndAssert(process.execPath, [
'-e', `const m = require(${JSON.stringify(
fixtures.path('es-modules', 'extensionless-cjs-module', 'index')
)}); if (m.hello !== 'world') throw new Error('expected CJS exports, got: ' + JSON.stringify(m))`,
], {
status: 0,
});
1 change: 1 addition & 0 deletions test/fixtures/es-modules/extensionless-cjs-module/index
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
module.exports = { hello: 'world' };
3 changes: 3 additions & 0 deletions test/fixtures/es-modules/extensionless-cjs-module/package.json
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
{
"type": "module"
}
Loading