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

esm: unflag extensionless ES module JavaScript and Wasm in module scope #49974

Closed
Closed
Show file tree
Hide file tree
Changes from 3 commits
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
8 changes: 6 additions & 2 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -1009,8 +1009,12 @@ _isImports_, _conditions_)
> 5. Let _packageURL_ be the result of **LOOKUP\_PACKAGE\_SCOPE**(_url_).
> 6. Let _pjson_ be the result of **READ\_PACKAGE\_JSON**(_packageURL_).
> 7. If _pjson?.type_ exists and is _"module"_, then
> 1. If _url_ ends in _".js"_, then
> 1. Return _"module"_.
> 1. If _url_ ends in _".js"_ or has no file extension, then
> 1. If `--experimental-wasm-modules` is enabled and the file at _url_
> contains the header for a WebAssembly module, then
> 1. Return _"wasm"_.
> 2. Otherwise,
> 1. Return _"module"_.
> 2. Return **undefined**.
> 8. Otherwise,
> 1. Return **undefined**.
Expand Down
8 changes: 1 addition & 7 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1760,13 +1760,7 @@ E('ERR_UNHANDLED_ERROR',
E('ERR_UNKNOWN_BUILTIN_MODULE', 'No such built-in module: %s', Error);
E('ERR_UNKNOWN_CREDENTIAL', '%s identifier does not exist: %s', Error);
E('ERR_UNKNOWN_ENCODING', 'Unknown encoding: %s', TypeError);
E('ERR_UNKNOWN_FILE_EXTENSION', (ext, path, suggestion) => {
let msg = `Unknown file extension "${ext}" for ${path}`;
if (suggestion) {
msg += `. ${suggestion}`;
}
return msg;
}, TypeError);
E('ERR_UNKNOWN_FILE_EXTENSION', 'Unknown file extension "%s" for %s', TypeError);
E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s for URL %s',
RangeError);
E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s', TypeError);
Expand Down
34 changes: 11 additions & 23 deletions lib/internal/modules/esm/get_format.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const {
StringPrototypeCharCodeAt,
StringPrototypeSlice,
} = primordials;
const { basename, relative } = require('path');
const { getOptionValue } = require('internal/options');
const {
extensionFormatMap,
Expand All @@ -22,7 +21,7 @@ const experimentalNetworkImports =
const defaultTypeFlag = getOptionValue('--experimental-default-type');
// The next line is where we flip the default to ES modules someday.
const defaultType = defaultTypeFlag === 'module' ? 'module' : 'commonjs';
const { getPackageType, getPackageScopeConfig } = require('internal/modules/esm/resolve');
const { getPackageType } = require('internal/modules/esm/resolve');
const { fileURLToPath } = require('internal/url');
const { ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes;

Expand Down Expand Up @@ -112,17 +111,16 @@ function getFileProtocolModuleFormat(url, context, ignoreErrors) {
if (defaultType === 'commonjs') { // Legacy behavior
if (packageType === 'none' || packageType === 'commonjs') {
return 'commonjs';
}
// If package type is `module`, fall through to the error case below
} else { // Else defaultType === 'module'
if (underNodeModules(url)) { // Exception for package scopes under `node_modules`
return 'commonjs';
}
if (packageType === 'none' || packageType === 'module') {
return getFormatOfExtensionlessFile(url);
} // Else packageType === 'commonjs'
return 'commonjs';
} // Else packageType === 'module'
return getFormatOfExtensionlessFile(url);
} // Else defaultType === 'module'
if (underNodeModules(url)) { // Exception for package scopes under `node_modules`
return packageType === 'module' ? 'module' : 'commonjs';
GeoffreyBooth marked this conversation as resolved.
Show resolved Hide resolved
}
if (packageType === 'none' || packageType === 'module') {
return getFormatOfExtensionlessFile(url);
} // Else packageType === 'commonjs'
return 'commonjs';
}

const format = extensionFormatMap[ext];
Expand All @@ -131,17 +129,7 @@ function getFileProtocolModuleFormat(url, context, ignoreErrors) {
// Explicit undefined return indicates load hook should rerun format check
if (ignoreErrors) { return undefined; }
const filepath = fileURLToPath(url);
let suggestion = '';
if (getPackageType(url) === 'module' && ext === '') {
const config = getPackageScopeConfig(url);
const fileBasename = basename(filepath);
const relativePath = StringPrototypeSlice(relative(config.pjsonPath, filepath), 1);
suggestion = 'Loading extensionless files is not supported inside of "type":"module" package.json contexts ' +
`without --experimental-default-type=module. The package.json file ${config.pjsonPath} caused this "type":"module" ` +
`context. Try changing ${filepath} to have a file extension. Note the "bin" field of package.json can point ` +
`to a file with an extension, for example {"type":"module","bin":{"${fileBasename}":"${relativePath}.js"}}`;
}
throw new ERR_UNKNOWN_FILE_EXTENSION(ext, filepath, suggestion);
throw new ERR_UNKNOWN_FILE_EXTENSION(ext, filepath);
}

/**
Expand Down
99 changes: 99 additions & 0 deletions test/es-module/test-esm-extensionless-esm-and-wasm.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
// Flags: --experimental-wasm-modules
import { mustNotCall, spawnPromisified } from '../common/index.mjs';
import * as fixtures from '../common/fixtures.mjs';
import { describe, it } from 'node:test';
import { match, ok, strictEqual } from 'node:assert';

describe('extensionless ES modules within a "type": "module" package scope', { concurrency: true }, () => {
it('should run as the entry point', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
fixtures.path('es-modules/package-type-module/noext-esm'),
]);

strictEqual(stderr, '');
strictEqual(stdout, 'executed\n');
strictEqual(code, 0);
strictEqual(signal, null);
});

it('should be importable', async () => {
const { default: defaultExport } =
await import(fixtures.fileURL('es-modules/package-type-module/noext-esm'));
strictEqual(defaultExport, 'module');
});

it('should be importable from a module scope under node_modules', async () => {
const { default: defaultExport } =
await import(fixtures.fileURL('es-modules/package-type-module/node_modules/dep-with-package-json-type-module/noext-esm'));

Check failure on line 27 in test/es-module/test-esm-extensionless-esm-and-wasm.mjs

View workflow job for this annotation

GitHub Actions / lint-js-and-md

This line has a length of 136. Maximum allowed is 120
strictEqual(defaultExport, 'module');
});
});
describe('extensionless Wasm modules within a "type": "module" package scope', { concurrency: true }, () => {
it('should run as the entry point', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
'--experimental-wasm-modules',
'--no-warnings',
fixtures.path('es-modules/package-type-module/noext-wasm'),
]);

strictEqual(stderr, '');
strictEqual(stdout, 'executed\n');
strictEqual(code, 0);
strictEqual(signal, null);
});

it('should be importable', async () => {
const { add } = await import(fixtures.fileURL('es-modules/package-type-module/noext-wasm'));
strictEqual(add(1, 2), 3);
});
});

describe('extensionless ES modules within no package scope', { concurrency: true }, () => {
// This succeeds with `--experimental-default-type=module`
it('should error as the entry point', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
fixtures.path('es-modules/noext-esm'),
]);

match(stderr, /SyntaxError/);
strictEqual(stdout, '');
strictEqual(code, 1);
strictEqual(signal, null);
});

// This succeeds with `--experimental-default-type=module`
it('should error on import', async () => {
try {
await import(fixtures.fileURL('es-modules/noext-esm'));
mustNotCall();
} catch (err) {
ok(err instanceof SyntaxError);
}
});
});

describe('extensionless Wasm within no package scope', { concurrency: true }, () => {
// This succeeds with `--experimental-default-type=module`
it('should error as the entry point', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
'--experimental-wasm-modules',
'--no-warnings',
fixtures.path('es-modules/noext-wasm'),
]);

match(stderr, /SyntaxError/);
strictEqual(stdout, '');
strictEqual(code, 1);
strictEqual(signal, null);
});

// This succeeds with `--experimental-default-type=module`
it('should error on import', async () => {
try {
await import(fixtures.fileURL('es-modules/noext-wasm'));
mustNotCall();
} catch (err) {
ok(err instanceof SyntaxError);
}
});
});
14 changes: 10 additions & 4 deletions test/es-module/test-esm-type-flag-package-scopes.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@
strictEqual(defaultExport, 'module');
});

it('should import an extensionless JavaScript file within a "type": "module" scope under node_modules', async () => {

Check failure on line 27 in test/es-module/test-esm-type-flag-package-scopes.mjs

View workflow job for this annotation

GitHub Actions / lint-js-and-md

This line has a length of 128. Maximum allowed is 120
const { default: defaultExport } =

Check failure on line 28 in test/es-module/test-esm-type-flag-package-scopes.mjs

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Expected indentation of 13 spaces but found 12
await import(fixtures.fileURL('es-modules/package-type-module/node_modules/dep-with-package-json-type-module/noext-esm'));

Check failure on line 29 in test/es-module/test-esm-type-flag-package-scopes.mjs

View workflow job for this annotation

GitHub Actions / lint-js-and-md

This line has a length of 144. Maximum allowed is 120
strictEqual(defaultExport, 'module');

Check failure on line 30 in test/es-module/test-esm-type-flag-package-scopes.mjs

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Expected indentation of 13 spaces but found 12
});

Check failure on line 31 in test/es-module/test-esm-type-flag-package-scopes.mjs

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Expected indentation of 11 spaces but found 10

it('should run as Wasm an extensionless Wasm file within a "type": "module" scope', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
'--experimental-default-type=module',
Expand Down Expand Up @@ -112,7 +118,7 @@
async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
'--experimental-default-type=module',
fixtures.path('es-modules/package-type-module/node_modules/dep-with-package-json/run.js'),
fixtures.path('es-modules/package-type-module/node_modules/dep-with-package-json-without-type/run.js'),
]);

strictEqual(stderr, '');
Expand All @@ -124,15 +130,15 @@
it(`should import as CommonJS a .js file within a package scope that has no defined "type" and is under
node_modules`, async () => {
const { default: defaultExport } =
await import(fixtures.fileURL('es-modules/package-type-module/node_modules/dep-with-package-json/run.js'));
await import(fixtures.fileURL('es-modules/package-type-module/node_modules/dep-with-package-json-without-type/run.js'));

Check failure on line 133 in test/es-module/test-esm-type-flag-package-scopes.mjs

View workflow job for this annotation

GitHub Actions / lint-js-and-md

This line has a length of 126. Maximum allowed is 120
strictEqual(defaultExport, 42);
});

it(`should run as CommonJS an extensionless JavaScript file within a package scope that has no defined "type" and is
under node_modules`, async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
'--experimental-default-type=module',
fixtures.path('es-modules/package-type-module/node_modules/dep-with-package-json/noext-cjs'),
fixtures.path('es-modules/package-type-module/node_modules/dep-with-package-json-without-type/noext-cjs'),
]);

strictEqual(stderr, '');
Expand All @@ -144,7 +150,7 @@
it(`should import as CommonJS an extensionless JavaScript file within a package scope that has no defined "type" and
is under node_modules`, async () => {
const { default: defaultExport } =
await import(fixtures.fileURL('es-modules/package-type-module/node_modules/dep-with-package-json/noext-cjs'));
await import(fixtures.fileURL('es-modules/package-type-module/node_modules/dep-with-package-json-without-type/noext-cjs'));

Check failure on line 153 in test/es-module/test-esm-type-flag-package-scopes.mjs

View workflow job for this annotation

GitHub Actions / lint-js-and-md

This line has a length of 129. Maximum allowed is 120
strictEqual(defaultExport, 42);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,11 @@ const { execPath } = require('node:process');
const { describe, it } = require('node:test');


// In a "type": "module" package scope, files with unknown extensions or no
// extensions should throw; both when used as a main entry point and also when
// referenced via `import`.
describe('ESM: extensionless and unknown specifiers', { concurrency: true }, () => {
// In a "type": "module" package scope, files with unknown extensions should throw;
// both when used as a main entry point and also when referenced via `import`.
describe('ESM: unknown specifiers', { concurrency: true }, () => {
for (
const fixturePath of [
'/es-modules/package-type-module/noext-esm',
'/es-modules/package-type-module/imports-noext.mjs',
'/es-modules/package-type-module/extension.unknown',
'/es-modules/package-type-module/imports-unknownext.mjs',
]
Expand All @@ -27,10 +24,6 @@ describe('ESM: extensionless and unknown specifiers', { concurrency: true }, ()
assert.strictEqual(signal, null);
assert.strictEqual(stdout, '');
assert.match(stderr, /ERR_UNKNOWN_FILE_EXTENSION/);
if (fixturePath.includes('noext')) {
// Check for explanation to users
assert.match(stderr, /extensionless/);
}
});
}
});

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading