Skip to content

Commit

Permalink
esm: empty ext from pkg type/main doesnt affect format
Browse files Browse the repository at this point in the history
This ensures files with unknown extensions like foo.bar are not
loaded as CJS/ESM when imported as a main entry point and makes
sure that those files would maintain the same format even if loaded
after the main entrypoint.

PR-URL: nodejs#31021
Reviewed-By: Guy Bedford <guybedford@gmail.com>
  • Loading branch information
bfarias-godaddy committed Dec 31, 2019
1 parent 3abe3f2 commit baa3621
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 10 deletions.
14 changes: 6 additions & 8 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -1154,15 +1154,13 @@ updates.
In the following algorithms, all subroutine errors are propagated as errors
of these top-level routines unless stated otherwise.
_isMain_ is **true** when resolving the Node.js application entry point.
_defaultEnv_ is the conditional environment name priority array,
`["node", "import"]`.
<details>
<summary>Resolver algorithm specification</summary>
**ESM_RESOLVE**(_specifier_, _parentURL_, _isMain_)
**ESM_RESOLVE**(_specifier_, _parentURL_)
> 1. Let _resolvedURL_ be **undefined**.
> 1. If _specifier_ is a valid URL, then
Expand All @@ -1183,7 +1181,7 @@ _defaultEnv_ is the conditional environment name priority array,
> 1. If the file at _resolvedURL_ does not exist, then
> 1. Throw a _Module Not Found_ error.
> 1. Set _resolvedURL_ to the real path of _resolvedURL_.
> 1. Let _format_ be the result of **ESM_FORMAT**(_resolvedURL_, _isMain_).
> 1. Let _format_ be the result of **ESM_FORMAT**(_resolvedURL_).
> 1. Load _resolvedURL_ as module format, _format_.
**PACKAGE_RESOLVE**(_packageSpecifier_, _parentURL_)
Expand Down Expand Up @@ -1334,20 +1332,20 @@ _defaultEnv_ is the conditional environment name priority array,
> 1. Return _resolved_.
> 1. Throw a _Module Not Found_ error.
**ESM_FORMAT**(_url_, _isMain_)
**ESM_FORMAT**(_url_)
> 1. Assert: _url_ corresponds to an existing file.
> 1. Assert: _url_ corresponds to an existing file pathname.
> 1. Let _pjson_ be the result of **READ_PACKAGE_SCOPE**(_url_).
> 1. If _url_ ends in _".mjs"_, then
> 1. Return _"module"_.
> 1. If _url_ ends in _".cjs"_, then
> 1. Return _"commonjs"_.
> 1. If _pjson?.type_ exists and is _"module"_, then
> 1. If _isMain_ is **true** or _url_ ends in _".js"_, then
> 1. If _url_ ends in _".js"_ or lacks a file extension, then
> 1. Return _"module"_.
> 1. Throw an _Unsupported File Extension_ error.
> 1. Otherwise,
> 1. If _isMain_ is **true**, then
> 1. If _url_ lacks a file extension, then
> 1. Return _"commonjs"_.
> 1. Throw an _Unsupported File Extension_ error.
Expand Down
7 changes: 5 additions & 2 deletions lib/internal/modules/esm/default_resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,12 @@ function resolve(specifier, parentURL) {
}

const ext = extname(url.pathname);
let format = extensionFormatMap[ext];
if (ext === '.js' || (!format && isMain))
let format;
if (ext === '.js' || ext === '') {
format = getPackageType(url.href) === TYPE_MODULE ? 'module' : 'commonjs';
} else {
format = extensionFormatMap[ext];
}
if (!format) {
if (experimentalSpeciferResolution === 'node') {
process.emitWarning(
Expand Down
81 changes: 81 additions & 0 deletions test/es-module/test-esm-unknown-main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
'use strict';

const common = require('../common');
const fixtures = require('../common/fixtures');
const { spawn } = require('child_process');
const assert = require('assert');

{
const entry = fixtures.path(
'/es-modules/package-type-module/extension.unknown'
);
const child = spawn(process.execPath, [entry]);
let stdout = '';
let stderr = '';
child.stderr.setEncoding('utf8');
child.stdout.setEncoding('utf8');
child.stdout.on('data', (data) => {
stdout += data;
});
child.stderr.on('data', (data) => {
stderr += data;
});
child.on('close', common.mustCall((code, signal) => {
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
assert.strictEqual(stdout, '');
assert.ok(stderr.indexOf('ERR_UNKNOWN_FILE_EXTENSION') !== -1);
}));
}
{
const entry = fixtures.path(
'/es-modules/package-type-module/imports-unknownext.mjs'
);
const child = spawn(process.execPath, [entry]);
let stdout = '';
let stderr = '';
child.stderr.setEncoding('utf8');
child.stdout.setEncoding('utf8');
child.stdout.on('data', (data) => {
stdout += data;
});
child.stderr.on('data', (data) => {
stderr += data;
});
child.on('close', common.mustCall((code, signal) => {
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
assert.strictEqual(stdout, '');
assert.ok(stderr.indexOf('ERR_UNKNOWN_FILE_EXTENSION') !== -1);
}));
}
{
const entry = fixtures.path('/es-modules/package-type-module/noext-esm');
const child = spawn(process.execPath, [entry]);
let stdout = '';
child.stdout.setEncoding('utf8');
child.stdout.on('data', (data) => {
stdout += data;
});
child.on('close', common.mustCall((code, signal) => {
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
assert.strictEqual(stdout, 'executed\n');
}));
}
{
const entry = fixtures.path(
'/es-modules/package-type-module/imports-noext.mjs'
);
const child = spawn(process.execPath, [entry]);
let stdout = '';
child.stdout.setEncoding('utf8');
child.stdout.on('data', (data) => {
stdout += data;
});
child.on('close', common.mustCall((code, signal) => {
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
assert.strictEqual(stdout, 'executed\n');
}));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
throw new Error('NO, NEVER');
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './noext-esm';
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './extension.unknown';

0 comments on commit baa3621

Please sign in to comment.