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 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
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: 2 additions & 6 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1760,12 +1760,8 @@ 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;
E('ERR_UNKNOWN_FILE_EXTENSION', (ext, path) => {
return `Unknown file extension "${ext}" for ${path}`;
}, TypeError);
E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s for URL %s',
GeoffreyBooth marked this conversation as resolved.
Show resolved Hide resolved
RangeError);
Expand Down
32 changes: 10 additions & 22 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'
} // Else packageType === 'module'
return getFormatOfExtensionlessFile(url);
} // 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';
GeoffreyBooth marked this conversation as resolved.
Show resolved Hide resolved
}

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
93 changes: 93 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,93 @@
// 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');
});
});
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);
}
});
});
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/);
}
});
}
});
Loading