Skip to content

Commit

Permalink
esm: improve check for ESM syntax
Browse files Browse the repository at this point in the history
  • Loading branch information
GeoffreyBooth committed Oct 10, 2023
1 parent a0a5b75 commit cc356ff
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 26 deletions.
14 changes: 12 additions & 2 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ const {
const {
getCjsConditions,
initializeCjsConditions,
hasEsmSyntax,
isModuleSyntaxError,
loadBuiltinModule,
makeRequireFunction,
normalizeReferrerURL,
Expand Down Expand Up @@ -1393,10 +1393,20 @@ Module._extensions['.js'] = function(module, filename) {
const pkg = packageJsonReader.readPackageScope(filename) || { __proto__: null };
// Function require shouldn't be used in ES modules.
if (pkg.data?.type === 'module') {
// This is an error path, because `require` of a `.js` file in a `"type": "module"` scope is not allowed.
const parent = moduleParentCache.get(module);
const parentPath = parent?.filename;
const packageJsonPath = path.resolve(pkg.path, 'package.json');
const usesEsm = hasEsmSyntax(content);

let usesEsm = false;
try {
internalCompileFunction(content, ['exports', 'require', 'module', '__filename', '__dirname'], { filename });
} catch (compilationError) {
if (isModuleSyntaxError(compilationError)) {
usesEsm = true;
}
}

const err = new ERR_REQUIRE_ESM(filename, usesEsm, parentPath,
packageJsonPath);
// Attempt to reconstruct the parent require frame.
Expand Down
7 changes: 2 additions & 5 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ const {
ArrayPrototypeMap,
Boolean,
JSONParse,
ObjectGetPrototypeOf,
ObjectPrototypeHasOwnProperty,
ObjectKeys,
ReflectApply,
Expand All @@ -15,7 +14,6 @@ const {
StringPrototypeReplaceAll,
StringPrototypeSlice,
StringPrototypeStartsWith,
SyntaxErrorPrototype,
globalThis: { WebAssembly },
} = primordials;

Expand All @@ -33,7 +31,7 @@ const assert = require('internal/assert');
const { readFileSync } = require('fs');
const { dirname, extname, isAbsolute } = require('path');
const {
hasEsmSyntax,
isModuleSyntaxError,
loadBuiltinModule,
stripBOM,
} = require('internal/modules/helpers');
Expand Down Expand Up @@ -168,8 +166,7 @@ translators.set('module', async function moduleStrategy(url, source, isMain) {
* @param {string} [filename] Useful only if `content` is unknown.
*/
function enrichCJSError(err, content, filename) {
if (err != null && ObjectGetPrototypeOf(err) === SyntaxErrorPrototype &&
hasEsmSyntax(content || readFileSync(filename, 'utf-8'))) {
if (isModuleSyntaxError(err)) {
// Emit the warning synchronously because we are in the middle of handling
// a SyntaxError that will throw and likely terminate the process before an
// asynchronous warning would be emitted.
Expand Down
36 changes: 17 additions & 19 deletions lib/internal/modules/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@
const {
ArrayPrototypeForEach,
ArrayPrototypeJoin,
ArrayPrototypeSome,
ObjectDefineProperty,
ObjectGetPrototypeOf,
ObjectPrototypeHasOwnProperty,
SafeMap,
SafeSet,
StringPrototypeCharCodeAt,
StringPrototypeIncludes,
StringPrototypeSlice,
StringPrototypeStartsWith,
SyntaxErrorPrototype,
} = primordials;
const {
ERR_INVALID_ARG_TYPE,
Expand Down Expand Up @@ -300,31 +301,28 @@ function normalizeReferrerURL(referrer) {
}

/**
* For error messages only, check if ESM syntax is in use.
* @param {string} code
* Check if the error is a syntax error due to ESM syntax in CommonJS.
* - `import` statements return an error with a message `Cannot use import statement outside a module`.
* - `export` statements return an error with a message `Unexpected token 'export'`.
* - `import.meta` returns an error with a message `Cannot use 'import.meta' outside a module`.
* Top-level `await` currently returns the same error message as when `await` is used in a sync function,
* so we don't use it as a disambiguation.
* Dynamic `import()` is permitted in CommonJS, so we don't use it as a disambiguation.
* @param {Error} err
*/
function hasEsmSyntax(code) {
debug('Checking for ESM syntax');
const parser = require('internal/deps/acorn/acorn/dist/acorn').Parser;
let root;
try {
root = parser.parse(code, { sourceType: 'module', ecmaVersion: 'latest' });
} catch {
return false;
}

return ArrayPrototypeSome(root.body, (stmt) =>
stmt.type === 'ExportDefaultDeclaration' ||
stmt.type === 'ExportNamedDeclaration' ||
stmt.type === 'ImportDeclaration' ||
stmt.type === 'ExportAllDeclaration');
function isModuleSyntaxError(err) {
return err != null && ObjectGetPrototypeOf(err) === SyntaxErrorPrototype && (
err.message === 'Cannot use import statement outside a module' ||
err.message === "Unexpected token 'export'" ||
err.message === "Cannot use 'import.meta' outside a module"
);
}

module.exports = {
addBuiltinLibsToObject,
getCjsConditions,
initializeCjsConditions,
hasEsmSyntax,
isModuleSyntaxError,
loadBuiltinModule,
makeRequireFunction,
normalizeReferrerURL,
Expand Down

0 comments on commit cc356ff

Please sign in to comment.