Skip to content

Commit

Permalink
esm: unflag extensionless javascript and wasm in module scope
Browse files Browse the repository at this point in the history
PR-URL: nodejs#49974
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
  • Loading branch information
GeoffreyBooth authored and targos committed Nov 11, 2023
1 parent ce76ac2 commit 823adc8
Show file tree
Hide file tree
Showing 14 changed files with 174 additions and 47 deletions.
8 changes: 6 additions & 2 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -1008,8 +1008,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 @@ -1686,13 +1686,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 @@ -25,7 +24,7 @@ const experimentalSpecifierResolution =
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 @@ -115,17 +114,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' ? getFormatOfExtensionlessFile(url) : 'commonjs';
}
if (packageType === 'none' || packageType === 'module') {
return getFormatOfExtensionlessFile(url);
} // Else packageType === 'commonjs'
return 'commonjs';
}

const format = extensionFormatMap[ext];
Expand All @@ -135,17 +133,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);
}

return getLegacyExtensionFormat(ext) ?? null;
Expand Down
106 changes: 106 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,106 @@
// 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'));
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);
});

it('should be importable from a module scope under node_modules', async () => {
const { add } = await import(fixtures.fileURL(
'es-modules/package-type-module/node_modules/dep-with-package-json-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);
}
});
});
25 changes: 21 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,14 @@ describe('the type flag should change the interpretation of certain files within
strictEqual(defaultExport, 'module');
});

it('should import an extensionless JavaScript file within a "type": "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'));
strictEqual(defaultExport, 'module');
});

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 All @@ -42,6 +50,13 @@ describe('the type flag should change the interpretation of certain files within
const { add } = await import(fixtures.fileURL('es-modules/package-type-module/noext-wasm'));
strictEqual(add(1, 2), 3);
});

it('should import an extensionless Wasm file within a "type": "module" scope under node_modules',
async () => {
const { add } = await import(fixtures.fileURL(
'es-modules/package-type-module/node_modules/dep-with-package-json-type-module/noext-wasm'));
strictEqual(add(1, 2), 3);
});
});

describe(`the type flag should change the interpretation of certain files within a package scope that lacks a
Expand Down Expand Up @@ -112,7 +127,7 @@ describe(`the type flag should NOT change the interpretation of certain files wi
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 +139,16 @@ describe(`the type flag should NOT change the interpretation of certain files wi
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'));
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 +160,8 @@ describe(`the type flag should NOT change the interpretation of certain files wi
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'));
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.

Binary file not shown.

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.

0 comments on commit 823adc8

Please sign in to comment.