From db1e0a2d1fc42d42cfb2a5ace1b9c48259f2bf56 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 7 Mar 2019 02:03:37 -0800 Subject: [PATCH 01/13] esm: --type=auto, to detect module type based on whether source contains import or export --- lib/internal/main/check_syntax.js | 4 ++- lib/internal/main/eval_stdin.js | 7 ++++- lib/internal/main/eval_string.js | 6 +++- lib/internal/modules/esm/default_resolve.js | 5 ++++ lib/internal/modules/esm/detect_type.js | 32 +++++++++++++++++++++ node.gyp | 1 + 6 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 lib/internal/modules/esm/detect_type.js diff --git a/lib/internal/main/check_syntax.js b/lib/internal/main/check_syntax.js index 6d335565ab..a7d0691675 100644 --- a/lib/internal/main/check_syntax.js +++ b/lib/internal/main/check_syntax.js @@ -60,7 +60,9 @@ function checkSyntax(source, filename) { if (experimentalModules) { let isModule = false; if (filename === '[stdin]' || filename === '[eval]') { - isModule = getOptionValue('--input-type') === 'module'; + const typeFlag = getOptionValue('--input-type'); + isModule = typeFlag === 'module' || (typeFlag === 'auto' && + require('internal/modules/esm/detect_type')(source) === 'module'); } else { const resolve = require('internal/modules/esm/default_resolve'); const { format } = resolve(pathToFileURL(filename).toString()); diff --git a/lib/internal/main/eval_stdin.js b/lib/internal/main/eval_stdin.js index 5c8f40b0e3..c3d8933fce 100644 --- a/lib/internal/main/eval_stdin.js +++ b/lib/internal/main/eval_stdin.js @@ -6,6 +6,8 @@ const { prepareMainThreadExecution } = require('internal/bootstrap/pre_execution'); +const { getOptionValue } = require('internal/options'); + const { evalModule, evalScript, @@ -17,7 +19,10 @@ markBootstrapComplete(); readStdin((code) => { process._eval = code; - if (require('internal/options').getOptionValue('--input-type') === 'module') + const typeFlag = getOptionValue('--input-type'); + if (typeFlag === 'module' || + (typeFlag === 'auto' && + require('internal/modules/esm/detect_type')(code) === 'module')) evalModule(process._eval); else evalScript('[stdin]', process._eval, process._breakFirstLine); diff --git a/lib/internal/main/eval_string.js b/lib/internal/main/eval_string.js index 4597d2a0bb..0fe40ff65d 100644 --- a/lib/internal/main/eval_string.js +++ b/lib/internal/main/eval_string.js @@ -14,7 +14,11 @@ const source = getOptionValue('--eval'); prepareMainThreadExecution(); addBuiltinLibsToObject(global); markBootstrapComplete(); -if (getOptionValue('--input-type') === 'module') + +const typeFlag = getOptionValue('--input-type'); +if (typeFlag === 'module' || + (typeFlag === 'auto' && + require('internal/modules/esm/detect_type')(code) === 'module')) evalModule(source); else evalScript('[eval]', source, process._breakFirstLine); diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index a83cf9c675..65c98566a6 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -94,6 +94,11 @@ function resolve(specifier, parentURL) { // entry point, etc.). throw new ERR_INPUT_TYPE_NOT_ALLOWED(); } + // --entry-type=auto detects an ESM .js file within a CommonJS scope. + if (isMain && format === 'commonjs' && typeFlag === 'auto') { + const source = readFileSync(fileURLToPath(url), 'utf8'); + format = require('internal/modules/esm/detect_type')(source); + } if (!format) { if (isMain) format = type === TYPE_MODULE ? 'module' : 'commonjs'; diff --git a/lib/internal/modules/esm/detect_type.js b/lib/internal/modules/esm/detect_type.js new file mode 100644 index 0000000000..6c4785bab5 --- /dev/null +++ b/lib/internal/modules/esm/detect_type.js @@ -0,0 +1,32 @@ +'use strict'; + +const acorn = require('internal/deps/acorn/acorn/dist/acorn'); + +// Detect the module type of a file: CommonJS or ES module. +// An ES module, for the purposes of this algorithm, is defined as any +// JavaScript file containing an import or export statement. +// Since our detection is so simple, we can avoid needing to use Acorn for a +// full parse; we can detect import or export statements just from the tokens. +function detectType(source) { + try { + let sawImport = false; + for (var token of acorn.tokenizer(source)) { + if (token.type.keyword === 'export') + return 'module'; + if (token.type.keyword === 'import') + sawImport = true; + if (sawImport) { + // Skip `import(`; look only for import statements, not expressions. + if (token.type.label === '(') + sawImport = false; + else + return 'module'; + } + } + } catch { + return 'commonjs'; + } + return 'commonjs'; +} + +module.exports = detectType; diff --git a/node.gyp b/node.gyp index 676be6e038..b4d57af317 100644 --- a/node.gyp +++ b/node.gyp @@ -150,6 +150,7 @@ 'lib/internal/modules/esm/loader.js', 'lib/internal/modules/esm/create_dynamic_module.js', 'lib/internal/modules/esm/default_resolve.js', + 'lib/internal/modules/esm/detect_type.js', 'lib/internal/modules/esm/module_job.js', 'lib/internal/modules/esm/module_map.js', 'lib/internal/modules/esm/translators.js', From 645f8f5bf4cc0607608fdec43c7663815c2e7bff Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 7 Mar 2019 22:33:42 -0800 Subject: [PATCH 02/13] esm: refactor import/export detection to use full parsing, as the tokens approach was too naive --- lib/internal/modules/esm/detect_type.js | 47 ++++++++++++++++--------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/lib/internal/modules/esm/detect_type.js b/lib/internal/modules/esm/detect_type.js index 6c4785bab5..f9a40078cb 100644 --- a/lib/internal/modules/esm/detect_type.js +++ b/lib/internal/modules/esm/detect_type.js @@ -1,31 +1,46 @@ 'use strict'; const acorn = require('internal/deps/acorn/acorn/dist/acorn'); +const walk = require('internal/deps/acorn/acorn-walk/dist/walk'); + +class BreakWalk extends Error {} // Detect the module type of a file: CommonJS or ES module. // An ES module, for the purposes of this algorithm, is defined as any // JavaScript file containing an import or export statement. -// Since our detection is so simple, we can avoid needing to use Acorn for a -// full parse; we can detect import or export statements just from the tokens. function detectType(source) { + let ast; try { - let sawImport = false; - for (var token of acorn.tokenizer(source)) { - if (token.type.keyword === 'export') - return 'module'; - if (token.type.keyword === 'import') - sawImport = true; - if (sawImport) { - // Skip `import(`; look only for import statements, not expressions. - if (token.type.label === '(') - sawImport = false; - else - return 'module'; - } - } + ast = acorn.parse(source, { + ecmaVersion: 10, // Highest supported version as of 2019 + allowReserved: true, + allowImportExportEverywhere: true, + allowAwaitOutsideFunction: true, + allowReturnOutsideFunction: true, // Required to parse CommonJS + allowHashBang: true // Required to parse hashbang scripts + }); } catch { return 'commonjs'; } + + // Walk the AST until we encounter an import or export statement. + // We put this in try/catch so that we can stop walking as soon as we find + // either. Acorn-walk has no built-in break/early termination functionality, + // so try/catch provides it (https://github.com/acornjs/acorn/issues/685). + const walkedIntoImportOrExport = () => { + throw new BreakWalk(); + }; + try { + walk.simple(ast, { + ImportDeclaration: walkedIntoImportOrExport, + ExportNamedDeclaration: walkedIntoImportOrExport, + ExportDefaultDeclaration: walkedIntoImportOrExport, + ExportAllDeclaration: walkedIntoImportOrExport + }); + } catch (err) { + if (err instanceof BreakWalk) + return 'module'; + } return 'commonjs'; } From 7976ad3343045208e6ebe25adf1123b9c6fc5a16 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Fri, 8 Mar 2019 00:17:04 -0800 Subject: [PATCH 03/13] esm: tests for --type=auto --- test/es-module/test-esm-type-auto.js | 37 +++++++++++++++++++ .../ambiguous-with-import-expression.js | 3 ++ .../cjs-with-import-expression.js | 5 +++ .../cjs-with-property-named-import.js | 3 ++ .../type-auto-scope/cjs-with-require.js | 3 ++ .../cjs-with-string-containing-import.js | 7 ++++ .../esm-with-export-statement.js | 6 +++ .../esm-with-import-statement.js | 2 + .../es-modules/type-auto-scope/package.json | 1 + .../type-auto-scope/print-version.js | 1 + 10 files changed, 68 insertions(+) create mode 100644 test/es-module/test-esm-type-auto.js create mode 100644 test/fixtures/es-modules/type-auto-scope/ambiguous-with-import-expression.js create mode 100644 test/fixtures/es-modules/type-auto-scope/cjs-with-import-expression.js create mode 100644 test/fixtures/es-modules/type-auto-scope/cjs-with-property-named-import.js create mode 100644 test/fixtures/es-modules/type-auto-scope/cjs-with-require.js create mode 100644 test/fixtures/es-modules/type-auto-scope/cjs-with-string-containing-import.js create mode 100644 test/fixtures/es-modules/type-auto-scope/esm-with-export-statement.js create mode 100644 test/fixtures/es-modules/type-auto-scope/esm-with-import-statement.js create mode 100644 test/fixtures/es-modules/type-auto-scope/package.json create mode 100644 test/fixtures/es-modules/type-auto-scope/print-version.js diff --git a/test/es-module/test-esm-type-auto.js b/test/es-module/test-esm-type-auto.js new file mode 100644 index 0000000000..0e96a8264a --- /dev/null +++ b/test/es-module/test-esm-type-auto.js @@ -0,0 +1,37 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const exec = require('child_process').execFile; + +const version = process.version; + +expect('esm-with-import-statement.js', version); +expect('esm-with-export-statement.js', version); + +expect('cjs-with-require.js', version); +expect('cjs-with-import-expression.js', version); +expect('cjs-with-property-named-import.js', version); +expect('cjs-with-string-containing-import.js', version); + +expect('print-version.js', version); +expect('ambiguous-with-import-expression.js', version); + +function expect(file, want) { + const argv = [ + require.resolve(`../fixtures/es-modules/type-auto-scope/${file}`) + ]; + ['--type=auto', '-a'].forEach((opt) => { + // TODO: Remove when --experimental-modules is unflagged + opt = `--experimental-modules ${opt}`; + const opts = { + env: Object.assign({}, process.env, { NODE_OPTIONS: opt }), + maxBuffer: 1e6, + }; + exec(process.execPath, argv, opts, + common.mustCall((err, stdout, stderr) => { + if (stdout.includes(want)) return; + assert.fail( + `For ${file}, failed to find ${want} in: <\n${stdout}\n>`); + })); + }); +} diff --git a/test/fixtures/es-modules/type-auto-scope/ambiguous-with-import-expression.js b/test/fixtures/es-modules/type-auto-scope/ambiguous-with-import-expression.js new file mode 100644 index 0000000000..8513a2fe09 --- /dev/null +++ b/test/fixtures/es-modules/type-auto-scope/ambiguous-with-import-expression.js @@ -0,0 +1,3 @@ +(async () => { + await import('./print-version.js'); +})(); diff --git a/test/fixtures/es-modules/type-auto-scope/cjs-with-import-expression.js b/test/fixtures/es-modules/type-auto-scope/cjs-with-import-expression.js new file mode 100644 index 0000000000..4ca064ff0f --- /dev/null +++ b/test/fixtures/es-modules/type-auto-scope/cjs-with-import-expression.js @@ -0,0 +1,5 @@ +const { version } = require('process'); + +(async () => { + await import('./print-version.js'); +})(); diff --git a/test/fixtures/es-modules/type-auto-scope/cjs-with-property-named-import.js b/test/fixtures/es-modules/type-auto-scope/cjs-with-property-named-import.js new file mode 100644 index 0000000000..e479ba3c2e --- /dev/null +++ b/test/fixtures/es-modules/type-auto-scope/cjs-with-property-named-import.js @@ -0,0 +1,3 @@ +global.import = 3; + +console.log(require('process').version); diff --git a/test/fixtures/es-modules/type-auto-scope/cjs-with-require.js b/test/fixtures/es-modules/type-auto-scope/cjs-with-require.js new file mode 100644 index 0000000000..195ad7956a --- /dev/null +++ b/test/fixtures/es-modules/type-auto-scope/cjs-with-require.js @@ -0,0 +1,3 @@ +const { version } = require('process'); + +console.log(version); diff --git a/test/fixtures/es-modules/type-auto-scope/cjs-with-string-containing-import.js b/test/fixtures/es-modules/type-auto-scope/cjs-with-string-containing-import.js new file mode 100644 index 0000000000..7f8319a90e --- /dev/null +++ b/test/fixtures/es-modules/type-auto-scope/cjs-with-string-containing-import.js @@ -0,0 +1,7 @@ +const { version } = require('process'); + +const sneakyString = ` +import { version } from 'process'; +`; + +console.log(version); diff --git a/test/fixtures/es-modules/type-auto-scope/esm-with-export-statement.js b/test/fixtures/es-modules/type-auto-scope/esm-with-export-statement.js new file mode 100644 index 0000000000..aed6f05d72 --- /dev/null +++ b/test/fixtures/es-modules/type-auto-scope/esm-with-export-statement.js @@ -0,0 +1,6 @@ +const version = process.version; + +export default version; + +console.log(version); + diff --git a/test/fixtures/es-modules/type-auto-scope/esm-with-import-statement.js b/test/fixtures/es-modules/type-auto-scope/esm-with-import-statement.js new file mode 100644 index 0000000000..fee4151c85 --- /dev/null +++ b/test/fixtures/es-modules/type-auto-scope/esm-with-import-statement.js @@ -0,0 +1,2 @@ +import { version } from 'process'; +console.log(version); diff --git a/test/fixtures/es-modules/type-auto-scope/package.json b/test/fixtures/es-modules/type-auto-scope/package.json new file mode 100644 index 0000000000..0967ef424b --- /dev/null +++ b/test/fixtures/es-modules/type-auto-scope/package.json @@ -0,0 +1 @@ +{} diff --git a/test/fixtures/es-modules/type-auto-scope/print-version.js b/test/fixtures/es-modules/type-auto-scope/print-version.js new file mode 100644 index 0000000000..557b2f103e --- /dev/null +++ b/test/fixtures/es-modules/type-auto-scope/print-version.js @@ -0,0 +1 @@ +console.log(process.version); From 943417ced78451161adae53ebc32688272151ea3 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Fri, 8 Mar 2019 01:38:22 -0800 Subject: [PATCH 04/13] esm: acorn doesn't support import(), so revert to tokenizer approach; more tests --- lib/internal/modules/esm/detect_type.js | 57 +++++++++---------- test/es-module/test-esm-type-auto.js | 3 + .../cjs-with-property-named-export.js | 11 ++++ .../cjs-with-property-named-import.js | 11 ++++ .../esm-with-import-expression.js | 5 ++ .../esm-with-indented-import-statement.js | 2 + 6 files changed, 58 insertions(+), 31 deletions(-) create mode 100644 test/fixtures/es-modules/type-auto-scope/cjs-with-property-named-export.js create mode 100644 test/fixtures/es-modules/type-auto-scope/esm-with-import-expression.js create mode 100644 test/fixtures/es-modules/type-auto-scope/esm-with-indented-import-statement.js diff --git a/lib/internal/modules/esm/detect_type.js b/lib/internal/modules/esm/detect_type.js index f9a40078cb..3a55633f77 100644 --- a/lib/internal/modules/esm/detect_type.js +++ b/lib/internal/modules/esm/detect_type.js @@ -1,46 +1,41 @@ 'use strict'; const acorn = require('internal/deps/acorn/acorn/dist/acorn'); -const walk = require('internal/deps/acorn/acorn-walk/dist/walk'); - -class BreakWalk extends Error {} // Detect the module type of a file: CommonJS or ES module. // An ES module, for the purposes of this algorithm, is defined as any // JavaScript file containing an import or export statement. +// Since our detection is so simple, we can avoid needing to use Acorn for a +// full parse; we can detect import or export statements just from the tokens. +// Also as of this writing, Acorn doesn't support import() expressions as they +// are only Stage 3; yet Node already supports them. function detectType(source) { - let ast; try { - ast = acorn.parse(source, { - ecmaVersion: 10, // Highest supported version as of 2019 - allowReserved: true, - allowImportExportEverywhere: true, - allowAwaitOutsideFunction: true, - allowReturnOutsideFunction: true, // Required to parse CommonJS - allowHashBang: true // Required to parse hashbang scripts - }); + let prevToken, prevPrevToken; + let sawKeyword = false; + for (const { type: token } of acorn.tokenizer(source)) { + if (token.keyword === 'import' || token.keyword === 'export') { + sawKeyword = true; + } else if (sawKeyword) { + // Skip `import(`; look only for import statements, not expressions. + // import() expressions are allowed in both CommonJS and ES modules. + if (token.label === '(' || + // Also ensure that the keyword we just saw wasn't an allowed use + // of a reserved word as a property name; see + // test/fixtures/es-modules/type-auto-scope/ + // cjs-with-property-named-import.js + (prevPrevToken && prevPrevToken.label === '.') || + token.label === ':') + sawKeyword = false; + else + return 'module'; + } + prevPrevToken = prevToken; + prevToken = token; + } } catch { return 'commonjs'; } - - // Walk the AST until we encounter an import or export statement. - // We put this in try/catch so that we can stop walking as soon as we find - // either. Acorn-walk has no built-in break/early termination functionality, - // so try/catch provides it (https://github.com/acornjs/acorn/issues/685). - const walkedIntoImportOrExport = () => { - throw new BreakWalk(); - }; - try { - walk.simple(ast, { - ImportDeclaration: walkedIntoImportOrExport, - ExportNamedDeclaration: walkedIntoImportOrExport, - ExportDefaultDeclaration: walkedIntoImportOrExport, - ExportAllDeclaration: walkedIntoImportOrExport - }); - } catch (err) { - if (err instanceof BreakWalk) - return 'module'; - } return 'commonjs'; } diff --git a/test/es-module/test-esm-type-auto.js b/test/es-module/test-esm-type-auto.js index 0e96a8264a..c3c690fdb9 100644 --- a/test/es-module/test-esm-type-auto.js +++ b/test/es-module/test-esm-type-auto.js @@ -7,10 +7,13 @@ const version = process.version; expect('esm-with-import-statement.js', version); expect('esm-with-export-statement.js', version); +expect('esm-with-import-expression.js', version); +expect('esm-with-indented-import-statement.js', version); expect('cjs-with-require.js', version); expect('cjs-with-import-expression.js', version); expect('cjs-with-property-named-import.js', version); +expect('cjs-with-property-named-export.js', version); expect('cjs-with-string-containing-import.js', version); expect('print-version.js', version); diff --git a/test/fixtures/es-modules/type-auto-scope/cjs-with-property-named-export.js b/test/fixtures/es-modules/type-auto-scope/cjs-with-property-named-export.js new file mode 100644 index 0000000000..4389e477ad --- /dev/null +++ b/test/fixtures/es-modules/type-auto-scope/cjs-with-property-named-export.js @@ -0,0 +1,11 @@ +// See ./cjs-with-property-named-import.js + +global.export = 3; + +global['export'] = 3; + +const obj = { +export: 3 // Specifically at column 0, to try to trick the detector +} + +console.log(require('process').version); diff --git a/test/fixtures/es-modules/type-auto-scope/cjs-with-property-named-import.js b/test/fixtures/es-modules/type-auto-scope/cjs-with-property-named-import.js index e479ba3c2e..8dec066037 100644 --- a/test/fixtures/es-modules/type-auto-scope/cjs-with-property-named-import.js +++ b/test/fixtures/es-modules/type-auto-scope/cjs-with-property-named-import.js @@ -1,3 +1,14 @@ +// In JavaScript, reserved words cannot be identifiers (the `foo` in `var foo`) +// but they can be properties (`obj.foo`). This file checks that the `import` +// reserved word isn't incorrectly detected as a keyword. For more info see: +// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar#Reserved_word_usage + global.import = 3; +global['import'] = 3; + +const obj = { +import: 3 // Specifically at column 0, to try to trick the detector +} + console.log(require('process').version); diff --git a/test/fixtures/es-modules/type-auto-scope/esm-with-import-expression.js b/test/fixtures/es-modules/type-auto-scope/esm-with-import-expression.js new file mode 100644 index 0000000000..83ed565b65 --- /dev/null +++ b/test/fixtures/es-modules/type-auto-scope/esm-with-import-expression.js @@ -0,0 +1,5 @@ +import { version } from 'process'; + +(async () => { + await import('./print-version.js'); +})(); diff --git a/test/fixtures/es-modules/type-auto-scope/esm-with-indented-import-statement.js b/test/fixtures/es-modules/type-auto-scope/esm-with-indented-import-statement.js new file mode 100644 index 0000000000..ca9d4edc0f --- /dev/null +++ b/test/fixtures/es-modules/type-auto-scope/esm-with-indented-import-statement.js @@ -0,0 +1,2 @@ + import { version } from 'process'; + console.log(version); From a833fdc744a40298a35e38032578153a6a5a153c Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Fri, 8 Mar 2019 12:52:45 -0800 Subject: [PATCH 05/13] esm: simplify logic in detect loop --- lib/internal/modules/esm/detect_type.js | 30 +++++++++++-------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/lib/internal/modules/esm/detect_type.js b/lib/internal/modules/esm/detect_type.js index 3a55633f77..3d3d44ef95 100644 --- a/lib/internal/modules/esm/detect_type.js +++ b/lib/internal/modules/esm/detect_type.js @@ -12,24 +12,20 @@ const acorn = require('internal/deps/acorn/acorn/dist/acorn'); function detectType(source) { try { let prevToken, prevPrevToken; - let sawKeyword = false; for (const { type: token } of acorn.tokenizer(source)) { - if (token.keyword === 'import' || token.keyword === 'export') { - sawKeyword = true; - } else if (sawKeyword) { - // Skip `import(`; look only for import statements, not expressions. - // import() expressions are allowed in both CommonJS and ES modules. - if (token.label === '(' || - // Also ensure that the keyword we just saw wasn't an allowed use - // of a reserved word as a property name; see - // test/fixtures/es-modules/type-auto-scope/ - // cjs-with-property-named-import.js - (prevPrevToken && prevPrevToken.label === '.') || - token.label === ':') - sawKeyword = false; - else - return 'module'; - } + if (prevToken && + // By definition import or export must be followed by another token. + (prevToken.keyword === 'import' || prevToken.keyword === 'export') && + // Skip `import(`; look only for import statements, not expressions. + // import() expressions are allowed in both CommonJS and ES modules. + token.label !== '(' && + // Also ensure that the keyword we just saw wasn't an allowed use + // of a reserved word as a property name; see + // test/fixtures/es-modules/type-auto-scope/ + // cjs-with-property-named-import.js + !(prevPrevToken && prevPrevToken.label === '.') && + token.label !== ':') + return 'module'; prevPrevToken = prevToken; prevToken = token; } From 0db82f36b115051a827246e34b6b5abfe01faf5c Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Fri, 8 Mar 2019 23:04:05 -0800 Subject: [PATCH 06/13] esm: fixup: code style --- test/es-module/test-esm-type-auto.js | 2 +- test/es-module/test-esm-type-flag-errors.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/es-module/test-esm-type-auto.js b/test/es-module/test-esm-type-auto.js index c3c690fdb9..5aae08f6f7 100644 --- a/test/es-module/test-esm-type-auto.js +++ b/test/es-module/test-esm-type-auto.js @@ -27,7 +27,7 @@ function expect(file, want) { // TODO: Remove when --experimental-modules is unflagged opt = `--experimental-modules ${opt}`; const opts = { - env: Object.assign({}, process.env, { NODE_OPTIONS: opt }), + env: { ...process.env, NODE_OPTIONS: opt }, maxBuffer: 1e6, }; exec(process.execPath, argv, opts, diff --git a/test/es-module/test-esm-type-flag-errors.js b/test/es-module/test-esm-type-flag-errors.js index 169118e018..f4ad2e6f4a 100644 --- a/test/es-module/test-esm-type-flag-errors.js +++ b/test/es-module/test-esm-type-flag-errors.js @@ -28,7 +28,7 @@ function expect(opt = '', inputFile, want, wantsError = false) { opt = `--experimental-modules ${opt}`; const argv = [inputFile]; const opts = { - env: Object.assign({}, process.env, { NODE_OPTIONS: opt }), + env: { ...process.env, NODE_OPTIONS: opt }, maxBuffer: 1e6, }; exec(process.execPath, argv, opts, common.mustCall((err, stdout, stderr) => { From 2c5ed92c6e9c235c6d150ee2a16b9ecfa3e76e2f Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sat, 9 Mar 2019 00:07:16 -0800 Subject: [PATCH 07/13] esm: --entry-type=auto should throw syntax errors directly --- lib/internal/main/check_syntax.js | 3 ++- lib/internal/main/eval_stdin.js | 2 +- lib/internal/main/eval_string.js | 2 +- lib/internal/modules/esm/default_resolve.js | 5 +++-- lib/internal/modules/esm/detect_type.js | 12 ++++++++++-- test/es-module/test-esm-type-auto.js | 10 +++++++++- .../es-modules/type-auto-scope/syntax-error-1.js | 1 + .../es-modules/type-auto-scope/syntax-error-2.js | 2 ++ 8 files changed, 29 insertions(+), 8 deletions(-) create mode 100644 test/fixtures/es-modules/type-auto-scope/syntax-error-1.js create mode 100644 test/fixtures/es-modules/type-auto-scope/syntax-error-2.js diff --git a/lib/internal/main/check_syntax.js b/lib/internal/main/check_syntax.js index a7d0691675..16c0b66e6a 100644 --- a/lib/internal/main/check_syntax.js +++ b/lib/internal/main/check_syntax.js @@ -62,7 +62,8 @@ function checkSyntax(source, filename) { if (filename === '[stdin]' || filename === '[eval]') { const typeFlag = getOptionValue('--input-type'); isModule = typeFlag === 'module' || (typeFlag === 'auto' && - require('internal/modules/esm/detect_type')(source) === 'module'); + require('internal/modules/esm/detect_type')(source, filename) === + 'module'); } else { const resolve = require('internal/modules/esm/default_resolve'); const { format } = resolve(pathToFileURL(filename).toString()); diff --git a/lib/internal/main/eval_stdin.js b/lib/internal/main/eval_stdin.js index c3d8933fce..0625b43bf2 100644 --- a/lib/internal/main/eval_stdin.js +++ b/lib/internal/main/eval_stdin.js @@ -22,7 +22,7 @@ readStdin((code) => { const typeFlag = getOptionValue('--input-type'); if (typeFlag === 'module' || (typeFlag === 'auto' && - require('internal/modules/esm/detect_type')(code) === 'module')) + require('internal/modules/esm/detect_type')(code, '[stdin]') === 'module')) evalModule(process._eval); else evalScript('[stdin]', process._eval, process._breakFirstLine); diff --git a/lib/internal/main/eval_string.js b/lib/internal/main/eval_string.js index 0fe40ff65d..9903942fa4 100644 --- a/lib/internal/main/eval_string.js +++ b/lib/internal/main/eval_string.js @@ -18,7 +18,7 @@ markBootstrapComplete(); const typeFlag = getOptionValue('--input-type'); if (typeFlag === 'module' || (typeFlag === 'auto' && - require('internal/modules/esm/detect_type')(code) === 'module')) + require('internal/modules/esm/detect_type')(source, '[eval]') === 'module')) evalModule(source); else evalScript('[eval]', source, process._breakFirstLine); diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index 65c98566a6..0cc90704ec 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -96,8 +96,9 @@ function resolve(specifier, parentURL) { } // --entry-type=auto detects an ESM .js file within a CommonJS scope. if (isMain && format === 'commonjs' && typeFlag === 'auto') { - const source = readFileSync(fileURLToPath(url), 'utf8'); - format = require('internal/modules/esm/detect_type')(source); + const filename = fileURLToPath(url); + const source = readFileSync(filename, 'utf8'); + format = require('internal/modules/esm/detect_type')(source, filename); } if (!format) { if (isMain) diff --git a/lib/internal/modules/esm/detect_type.js b/lib/internal/modules/esm/detect_type.js index 3d3d44ef95..bbb5f0a8a7 100644 --- a/lib/internal/modules/esm/detect_type.js +++ b/lib/internal/modules/esm/detect_type.js @@ -1,6 +1,9 @@ 'use strict'; const acorn = require('internal/deps/acorn/acorn/dist/acorn'); +const { + stripShebang, stripBOM +} = require('internal/modules/cjs/helpers'); // Detect the module type of a file: CommonJS or ES module. // An ES module, for the purposes of this algorithm, is defined as any @@ -9,7 +12,9 @@ const acorn = require('internal/deps/acorn/acorn/dist/acorn'); // full parse; we can detect import or export statements just from the tokens. // Also as of this writing, Acorn doesn't support import() expressions as they // are only Stage 3; yet Node already supports them. -function detectType(source) { +function detectType(source, filename) { + source = stripShebang(source); + source = stripBOM(source); try { let prevToken, prevPrevToken; for (const { type: token } of acorn.tokenizer(source)) { @@ -30,7 +35,10 @@ function detectType(source) { prevToken = token; } } catch { - return 'commonjs'; + // If the tokenizer threw, there's a syntax error. + // Compile the script, this will throw with an informative error. + const vm = require('vm'); + new vm.Script(source, { displayErrors: true, filename }); } return 'commonjs'; } diff --git a/test/es-module/test-esm-type-auto.js b/test/es-module/test-esm-type-auto.js index 5aae08f6f7..e59fbad86d 100644 --- a/test/es-module/test-esm-type-auto.js +++ b/test/es-module/test-esm-type-auto.js @@ -19,7 +19,10 @@ expect('cjs-with-string-containing-import.js', version); expect('print-version.js', version); expect('ambiguous-with-import-expression.js', version); -function expect(file, want) { +expect('syntax-error-1.js', 'SyntaxError', true); +expect('syntax-error-2.js', 'SyntaxError', true); + +function expect(file, want, wantsError = false) { const argv = [ require.resolve(`../fixtures/es-modules/type-auto-scope/${file}`) ]; @@ -32,6 +35,11 @@ function expect(file, want) { }; exec(process.execPath, argv, opts, common.mustCall((err, stdout, stderr) => { + if (wantsError) { + stdout = stderr; + } else { + assert.ifError(err); + } if (stdout.includes(want)) return; assert.fail( `For ${file}, failed to find ${want} in: <\n${stdout}\n>`); diff --git a/test/fixtures/es-modules/type-auto-scope/syntax-error-1.js b/test/fixtures/es-modules/type-auto-scope/syntax-error-1.js new file mode 100644 index 0000000000..1f5d64097b --- /dev/null +++ b/test/fixtures/es-modules/type-auto-scope/syntax-error-1.js @@ -0,0 +1 @@ +export \ No newline at end of file diff --git a/test/fixtures/es-modules/type-auto-scope/syntax-error-2.js b/test/fixtures/es-modules/type-auto-scope/syntax-error-2.js new file mode 100644 index 0000000000..11d0e77b48 --- /dev/null +++ b/test/fixtures/es-modules/type-auto-scope/syntax-error-2.js @@ -0,0 +1,2 @@ +const str = 'import +var foo = 3; \ No newline at end of file From 92c963b055cdee22c9f80bc03f1a820bc131950f Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Sat, 9 Mar 2019 00:20:41 -0800 Subject: [PATCH 08/13] esm: fixup: code style Co-Authored-By: GeoffreyBooth --- lib/internal/modules/esm/detect_type.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/internal/modules/esm/detect_type.js b/lib/internal/modules/esm/detect_type.js index bbb5f0a8a7..e1fb50a9fd 100644 --- a/lib/internal/modules/esm/detect_type.js +++ b/lib/internal/modules/esm/detect_type.js @@ -2,7 +2,8 @@ const acorn = require('internal/deps/acorn/acorn/dist/acorn'); const { - stripShebang, stripBOM + stripShebang, + stripBOM, } = require('internal/modules/cjs/helpers'); // Detect the module type of a file: CommonJS or ES module. From e537ade8cafe4744ea2918f001a0da8ddd6f0f83 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Tue, 2 Apr 2019 23:57:02 -0700 Subject: [PATCH 09/13] esm: fix tests --- lib/internal/modules/esm/default_resolve.js | 2 +- lib/internal/modules/esm/detect_type.js | 2 +- src/node_options.cc | 6 ++-- ...pe-auto.js => test-esm-entry-type-auto.js} | 36 +++++++++---------- .../ambiguous-with-import-expression.js | 0 .../cjs-with-import-expression.js | 0 .../cjs-with-property-named-export.js | 0 .../cjs-with-property-named-import.js | 0 .../cjs-with-require.js | 0 .../cjs-with-string-containing-import.js | 0 .../esm-with-export-statement.js | 0 .../esm-with-import-expression.js | 0 .../esm-with-import-statement.js | 0 .../esm-with-indented-import-statement.js | 0 .../package.json | 0 .../print-version.js | 0 .../syntax-error-1.js | 0 .../syntax-error-2.js | 0 18 files changed, 23 insertions(+), 23 deletions(-) rename test/es-module/{test-esm-type-auto.js => test-esm-entry-type-auto.js} (60%) rename test/fixtures/es-modules/{type-auto-scope => entry-type-auto-scope}/ambiguous-with-import-expression.js (100%) rename test/fixtures/es-modules/{type-auto-scope => entry-type-auto-scope}/cjs-with-import-expression.js (100%) rename test/fixtures/es-modules/{type-auto-scope => entry-type-auto-scope}/cjs-with-property-named-export.js (100%) rename test/fixtures/es-modules/{type-auto-scope => entry-type-auto-scope}/cjs-with-property-named-import.js (100%) rename test/fixtures/es-modules/{type-auto-scope => entry-type-auto-scope}/cjs-with-require.js (100%) rename test/fixtures/es-modules/{type-auto-scope => entry-type-auto-scope}/cjs-with-string-containing-import.js (100%) rename test/fixtures/es-modules/{type-auto-scope => entry-type-auto-scope}/esm-with-export-statement.js (100%) rename test/fixtures/es-modules/{type-auto-scope => entry-type-auto-scope}/esm-with-import-expression.js (100%) rename test/fixtures/es-modules/{type-auto-scope => entry-type-auto-scope}/esm-with-import-statement.js (100%) rename test/fixtures/es-modules/{type-auto-scope => entry-type-auto-scope}/esm-with-indented-import-statement.js (100%) rename test/fixtures/es-modules/{type-auto-scope => entry-type-auto-scope}/package.json (100%) rename test/fixtures/es-modules/{type-auto-scope => entry-type-auto-scope}/print-version.js (100%) rename test/fixtures/es-modules/{type-auto-scope => entry-type-auto-scope}/syntax-error-1.js (100%) rename test/fixtures/es-modules/{type-auto-scope => entry-type-auto-scope}/syntax-error-2.js (100%) diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index 0cc90704ec..6fd2e5b2d0 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -3,7 +3,7 @@ const internalFS = require('internal/fs/utils'); const { NativeModule } = require('internal/bootstrap/loaders'); const { extname } = require('path'); -const { realpathSync } = require('fs'); +const { readFileSync, realpathSync } = require('fs'); const { getOptionValue } = require('internal/options'); const preserveSymlinks = getOptionValue('--preserve-symlinks'); diff --git a/lib/internal/modules/esm/detect_type.js b/lib/internal/modules/esm/detect_type.js index e1fb50a9fd..54d1955be7 100644 --- a/lib/internal/modules/esm/detect_type.js +++ b/lib/internal/modules/esm/detect_type.js @@ -27,7 +27,7 @@ function detectType(source, filename) { token.label !== '(' && // Also ensure that the keyword we just saw wasn't an allowed use // of a reserved word as a property name; see - // test/fixtures/es-modules/type-auto-scope/ + // test/fixtures/es-modules/entry-type-auto-scope/ // cjs-with-property-named-import.js !(prevPrevToken && prevPrevToken.label === '.') && token.label !== ':') diff --git a/src/node_options.cc b/src/node_options.cc index 942280347e..010ede5841 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -112,8 +112,10 @@ void EnvironmentOptions::CheckOptions(std::vector* errors) { errors->push_back("--input-type requires " "--experimental-modules to be enabled"); } - if (module_type != "commonjs" && module_type != "module") { - errors->push_back("--input-type must be \"module\" or \"commonjs\""); + if (module_type != "commonjs" && module_type != "module" && + module_type != "auto") { + errors->push_back("--input-type must be " + "\"module\" or \"commonjs\" or \"auto\""); } } diff --git a/test/es-module/test-esm-type-auto.js b/test/es-module/test-esm-entry-type-auto.js similarity index 60% rename from test/es-module/test-esm-type-auto.js rename to test/es-module/test-esm-entry-type-auto.js index e59fbad86d..92fb4f56fb 100644 --- a/test/es-module/test-esm-type-auto.js +++ b/test/es-module/test-esm-entry-type-auto.js @@ -24,25 +24,23 @@ expect('syntax-error-2.js', 'SyntaxError', true); function expect(file, want, wantsError = false) { const argv = [ - require.resolve(`../fixtures/es-modules/type-auto-scope/${file}`) + require.resolve(`../fixtures/es-modules/entry-type-auto-scope/${file}`) ]; - ['--type=auto', '-a'].forEach((opt) => { + const opts = { // TODO: Remove when --experimental-modules is unflagged - opt = `--experimental-modules ${opt}`; - const opts = { - env: { ...process.env, NODE_OPTIONS: opt }, - maxBuffer: 1e6, - }; - exec(process.execPath, argv, opts, - common.mustCall((err, stdout, stderr) => { - if (wantsError) { - stdout = stderr; - } else { - assert.ifError(err); - } - if (stdout.includes(want)) return; - assert.fail( - `For ${file}, failed to find ${want} in: <\n${stdout}\n>`); - })); - }); + env: { ...process.env, + NODE_OPTIONS: '--experimental-modules --entry-type=auto' }, + maxBuffer: 1e6, + }; + exec(process.execPath, argv, opts, + common.mustCall((err, stdout, stderr) => { + if (wantsError) { + stdout = stderr; + } else { + assert.ifError(err); + } + if (stdout.includes(want)) return; + assert.fail( + `For ${file}, failed to find ${want} in: <\n${stdout}\n>`); + })); } diff --git a/test/fixtures/es-modules/type-auto-scope/ambiguous-with-import-expression.js b/test/fixtures/es-modules/entry-type-auto-scope/ambiguous-with-import-expression.js similarity index 100% rename from test/fixtures/es-modules/type-auto-scope/ambiguous-with-import-expression.js rename to test/fixtures/es-modules/entry-type-auto-scope/ambiguous-with-import-expression.js diff --git a/test/fixtures/es-modules/type-auto-scope/cjs-with-import-expression.js b/test/fixtures/es-modules/entry-type-auto-scope/cjs-with-import-expression.js similarity index 100% rename from test/fixtures/es-modules/type-auto-scope/cjs-with-import-expression.js rename to test/fixtures/es-modules/entry-type-auto-scope/cjs-with-import-expression.js diff --git a/test/fixtures/es-modules/type-auto-scope/cjs-with-property-named-export.js b/test/fixtures/es-modules/entry-type-auto-scope/cjs-with-property-named-export.js similarity index 100% rename from test/fixtures/es-modules/type-auto-scope/cjs-with-property-named-export.js rename to test/fixtures/es-modules/entry-type-auto-scope/cjs-with-property-named-export.js diff --git a/test/fixtures/es-modules/type-auto-scope/cjs-with-property-named-import.js b/test/fixtures/es-modules/entry-type-auto-scope/cjs-with-property-named-import.js similarity index 100% rename from test/fixtures/es-modules/type-auto-scope/cjs-with-property-named-import.js rename to test/fixtures/es-modules/entry-type-auto-scope/cjs-with-property-named-import.js diff --git a/test/fixtures/es-modules/type-auto-scope/cjs-with-require.js b/test/fixtures/es-modules/entry-type-auto-scope/cjs-with-require.js similarity index 100% rename from test/fixtures/es-modules/type-auto-scope/cjs-with-require.js rename to test/fixtures/es-modules/entry-type-auto-scope/cjs-with-require.js diff --git a/test/fixtures/es-modules/type-auto-scope/cjs-with-string-containing-import.js b/test/fixtures/es-modules/entry-type-auto-scope/cjs-with-string-containing-import.js similarity index 100% rename from test/fixtures/es-modules/type-auto-scope/cjs-with-string-containing-import.js rename to test/fixtures/es-modules/entry-type-auto-scope/cjs-with-string-containing-import.js diff --git a/test/fixtures/es-modules/type-auto-scope/esm-with-export-statement.js b/test/fixtures/es-modules/entry-type-auto-scope/esm-with-export-statement.js similarity index 100% rename from test/fixtures/es-modules/type-auto-scope/esm-with-export-statement.js rename to test/fixtures/es-modules/entry-type-auto-scope/esm-with-export-statement.js diff --git a/test/fixtures/es-modules/type-auto-scope/esm-with-import-expression.js b/test/fixtures/es-modules/entry-type-auto-scope/esm-with-import-expression.js similarity index 100% rename from test/fixtures/es-modules/type-auto-scope/esm-with-import-expression.js rename to test/fixtures/es-modules/entry-type-auto-scope/esm-with-import-expression.js diff --git a/test/fixtures/es-modules/type-auto-scope/esm-with-import-statement.js b/test/fixtures/es-modules/entry-type-auto-scope/esm-with-import-statement.js similarity index 100% rename from test/fixtures/es-modules/type-auto-scope/esm-with-import-statement.js rename to test/fixtures/es-modules/entry-type-auto-scope/esm-with-import-statement.js diff --git a/test/fixtures/es-modules/type-auto-scope/esm-with-indented-import-statement.js b/test/fixtures/es-modules/entry-type-auto-scope/esm-with-indented-import-statement.js similarity index 100% rename from test/fixtures/es-modules/type-auto-scope/esm-with-indented-import-statement.js rename to test/fixtures/es-modules/entry-type-auto-scope/esm-with-indented-import-statement.js diff --git a/test/fixtures/es-modules/type-auto-scope/package.json b/test/fixtures/es-modules/entry-type-auto-scope/package.json similarity index 100% rename from test/fixtures/es-modules/type-auto-scope/package.json rename to test/fixtures/es-modules/entry-type-auto-scope/package.json diff --git a/test/fixtures/es-modules/type-auto-scope/print-version.js b/test/fixtures/es-modules/entry-type-auto-scope/print-version.js similarity index 100% rename from test/fixtures/es-modules/type-auto-scope/print-version.js rename to test/fixtures/es-modules/entry-type-auto-scope/print-version.js diff --git a/test/fixtures/es-modules/type-auto-scope/syntax-error-1.js b/test/fixtures/es-modules/entry-type-auto-scope/syntax-error-1.js similarity index 100% rename from test/fixtures/es-modules/type-auto-scope/syntax-error-1.js rename to test/fixtures/es-modules/entry-type-auto-scope/syntax-error-1.js diff --git a/test/fixtures/es-modules/type-auto-scope/syntax-error-2.js b/test/fixtures/es-modules/entry-type-auto-scope/syntax-error-2.js similarity index 100% rename from test/fixtures/es-modules/type-auto-scope/syntax-error-2.js rename to test/fixtures/es-modules/entry-type-auto-scope/syntax-error-2.js From 6888e0b82e9f23167d59ab10c0491301c53bc35a Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Wed, 3 Apr 2019 20:01:47 -0700 Subject: [PATCH 10/13] esm: document auto in the ESM docs --- doc/api/esm.md | 7 +++++++ lib/internal/modules/esm/default_resolve.js | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 36dba24e29..16ac1c22dd 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -174,6 +174,13 @@ For completeness there is also `--input-type=commonjs`, for explicitly running string input as CommonJS. This is the default behavior if `--input-type` is unspecified. +There is also `--input-type=auto`, which configures Node.js to interpret string +input as an ES module if Node.js finds an `import` or `export` statement in the +source code. (Note that dynamic `import()` expressions are different from +`import` statements; `import()` is allowed in both CommonJS and ES modules.) If +no `import` or `export` statements are found, the string is interpreted as +CommonJS. + ## Package Entry Points The `package.json` `"main"` field defines the entry point for a package, diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index 6fd2e5b2d0..87025d67c8 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -94,7 +94,7 @@ function resolve(specifier, parentURL) { // entry point, etc.). throw new ERR_INPUT_TYPE_NOT_ALLOWED(); } - // --entry-type=auto detects an ESM .js file within a CommonJS scope. + // --input-type=auto detects an ESM .js file within a CommonJS scope. if (isMain && format === 'commonjs' && typeFlag === 'auto') { const filename = fileURLToPath(url); const source = readFileSync(filename, 'utf8'); From bbbf03d9c3d83689225c7b1514ce8bb063485a1a Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Wed, 3 Apr 2019 23:47:46 -0700 Subject: [PATCH 11/13] esm: rename tests for --input-type=auto --- lib/internal/modules/esm/detect_type.js | 2 +- ...est-esm-entry-type-auto.js => test-esm-input-type-auto.js} | 4 ++-- .../ambiguous-with-import-expression.js | 0 .../cjs-with-import-expression.js | 0 .../cjs-with-property-named-export.js | 0 .../cjs-with-property-named-import.js | 0 .../cjs-with-require.js | 0 .../cjs-with-string-containing-import.js | 0 .../esm-with-export-statement.js | 0 .../esm-with-import-expression.js | 0 .../esm-with-import-statement.js | 0 .../esm-with-indented-import-statement.js | 0 .../package.json | 0 .../print-version.js | 0 .../syntax-error-1.js | 0 .../syntax-error-2.js | 0 16 files changed, 3 insertions(+), 3 deletions(-) rename test/es-module/{test-esm-entry-type-auto.js => test-esm-input-type-auto.js} (91%) rename test/fixtures/es-modules/{entry-type-auto-scope => input-type-auto-scope}/ambiguous-with-import-expression.js (100%) rename test/fixtures/es-modules/{entry-type-auto-scope => input-type-auto-scope}/cjs-with-import-expression.js (100%) rename test/fixtures/es-modules/{entry-type-auto-scope => input-type-auto-scope}/cjs-with-property-named-export.js (100%) rename test/fixtures/es-modules/{entry-type-auto-scope => input-type-auto-scope}/cjs-with-property-named-import.js (100%) rename test/fixtures/es-modules/{entry-type-auto-scope => input-type-auto-scope}/cjs-with-require.js (100%) rename test/fixtures/es-modules/{entry-type-auto-scope => input-type-auto-scope}/cjs-with-string-containing-import.js (100%) rename test/fixtures/es-modules/{entry-type-auto-scope => input-type-auto-scope}/esm-with-export-statement.js (100%) rename test/fixtures/es-modules/{entry-type-auto-scope => input-type-auto-scope}/esm-with-import-expression.js (100%) rename test/fixtures/es-modules/{entry-type-auto-scope => input-type-auto-scope}/esm-with-import-statement.js (100%) rename test/fixtures/es-modules/{entry-type-auto-scope => input-type-auto-scope}/esm-with-indented-import-statement.js (100%) rename test/fixtures/es-modules/{entry-type-auto-scope => input-type-auto-scope}/package.json (100%) rename test/fixtures/es-modules/{entry-type-auto-scope => input-type-auto-scope}/print-version.js (100%) rename test/fixtures/es-modules/{entry-type-auto-scope => input-type-auto-scope}/syntax-error-1.js (100%) rename test/fixtures/es-modules/{entry-type-auto-scope => input-type-auto-scope}/syntax-error-2.js (100%) diff --git a/lib/internal/modules/esm/detect_type.js b/lib/internal/modules/esm/detect_type.js index 54d1955be7..4fac79aac5 100644 --- a/lib/internal/modules/esm/detect_type.js +++ b/lib/internal/modules/esm/detect_type.js @@ -27,7 +27,7 @@ function detectType(source, filename) { token.label !== '(' && // Also ensure that the keyword we just saw wasn't an allowed use // of a reserved word as a property name; see - // test/fixtures/es-modules/entry-type-auto-scope/ + // test/fixtures/es-modules/input-type-auto-scope/ // cjs-with-property-named-import.js !(prevPrevToken && prevPrevToken.label === '.') && token.label !== ':') diff --git a/test/es-module/test-esm-entry-type-auto.js b/test/es-module/test-esm-input-type-auto.js similarity index 91% rename from test/es-module/test-esm-entry-type-auto.js rename to test/es-module/test-esm-input-type-auto.js index 92fb4f56fb..f9eaaaf3f6 100644 --- a/test/es-module/test-esm-entry-type-auto.js +++ b/test/es-module/test-esm-input-type-auto.js @@ -24,12 +24,12 @@ expect('syntax-error-2.js', 'SyntaxError', true); function expect(file, want, wantsError = false) { const argv = [ - require.resolve(`../fixtures/es-modules/entry-type-auto-scope/${file}`) + require.resolve(`../fixtures/es-modules/input-type-auto-scope/${file}`) ]; const opts = { // TODO: Remove when --experimental-modules is unflagged env: { ...process.env, - NODE_OPTIONS: '--experimental-modules --entry-type=auto' }, + NODE_OPTIONS: '--experimental-modules --input-type=auto' }, maxBuffer: 1e6, }; exec(process.execPath, argv, opts, diff --git a/test/fixtures/es-modules/entry-type-auto-scope/ambiguous-with-import-expression.js b/test/fixtures/es-modules/input-type-auto-scope/ambiguous-with-import-expression.js similarity index 100% rename from test/fixtures/es-modules/entry-type-auto-scope/ambiguous-with-import-expression.js rename to test/fixtures/es-modules/input-type-auto-scope/ambiguous-with-import-expression.js diff --git a/test/fixtures/es-modules/entry-type-auto-scope/cjs-with-import-expression.js b/test/fixtures/es-modules/input-type-auto-scope/cjs-with-import-expression.js similarity index 100% rename from test/fixtures/es-modules/entry-type-auto-scope/cjs-with-import-expression.js rename to test/fixtures/es-modules/input-type-auto-scope/cjs-with-import-expression.js diff --git a/test/fixtures/es-modules/entry-type-auto-scope/cjs-with-property-named-export.js b/test/fixtures/es-modules/input-type-auto-scope/cjs-with-property-named-export.js similarity index 100% rename from test/fixtures/es-modules/entry-type-auto-scope/cjs-with-property-named-export.js rename to test/fixtures/es-modules/input-type-auto-scope/cjs-with-property-named-export.js diff --git a/test/fixtures/es-modules/entry-type-auto-scope/cjs-with-property-named-import.js b/test/fixtures/es-modules/input-type-auto-scope/cjs-with-property-named-import.js similarity index 100% rename from test/fixtures/es-modules/entry-type-auto-scope/cjs-with-property-named-import.js rename to test/fixtures/es-modules/input-type-auto-scope/cjs-with-property-named-import.js diff --git a/test/fixtures/es-modules/entry-type-auto-scope/cjs-with-require.js b/test/fixtures/es-modules/input-type-auto-scope/cjs-with-require.js similarity index 100% rename from test/fixtures/es-modules/entry-type-auto-scope/cjs-with-require.js rename to test/fixtures/es-modules/input-type-auto-scope/cjs-with-require.js diff --git a/test/fixtures/es-modules/entry-type-auto-scope/cjs-with-string-containing-import.js b/test/fixtures/es-modules/input-type-auto-scope/cjs-with-string-containing-import.js similarity index 100% rename from test/fixtures/es-modules/entry-type-auto-scope/cjs-with-string-containing-import.js rename to test/fixtures/es-modules/input-type-auto-scope/cjs-with-string-containing-import.js diff --git a/test/fixtures/es-modules/entry-type-auto-scope/esm-with-export-statement.js b/test/fixtures/es-modules/input-type-auto-scope/esm-with-export-statement.js similarity index 100% rename from test/fixtures/es-modules/entry-type-auto-scope/esm-with-export-statement.js rename to test/fixtures/es-modules/input-type-auto-scope/esm-with-export-statement.js diff --git a/test/fixtures/es-modules/entry-type-auto-scope/esm-with-import-expression.js b/test/fixtures/es-modules/input-type-auto-scope/esm-with-import-expression.js similarity index 100% rename from test/fixtures/es-modules/entry-type-auto-scope/esm-with-import-expression.js rename to test/fixtures/es-modules/input-type-auto-scope/esm-with-import-expression.js diff --git a/test/fixtures/es-modules/entry-type-auto-scope/esm-with-import-statement.js b/test/fixtures/es-modules/input-type-auto-scope/esm-with-import-statement.js similarity index 100% rename from test/fixtures/es-modules/entry-type-auto-scope/esm-with-import-statement.js rename to test/fixtures/es-modules/input-type-auto-scope/esm-with-import-statement.js diff --git a/test/fixtures/es-modules/entry-type-auto-scope/esm-with-indented-import-statement.js b/test/fixtures/es-modules/input-type-auto-scope/esm-with-indented-import-statement.js similarity index 100% rename from test/fixtures/es-modules/entry-type-auto-scope/esm-with-indented-import-statement.js rename to test/fixtures/es-modules/input-type-auto-scope/esm-with-indented-import-statement.js diff --git a/test/fixtures/es-modules/entry-type-auto-scope/package.json b/test/fixtures/es-modules/input-type-auto-scope/package.json similarity index 100% rename from test/fixtures/es-modules/entry-type-auto-scope/package.json rename to test/fixtures/es-modules/input-type-auto-scope/package.json diff --git a/test/fixtures/es-modules/entry-type-auto-scope/print-version.js b/test/fixtures/es-modules/input-type-auto-scope/print-version.js similarity index 100% rename from test/fixtures/es-modules/entry-type-auto-scope/print-version.js rename to test/fixtures/es-modules/input-type-auto-scope/print-version.js diff --git a/test/fixtures/es-modules/entry-type-auto-scope/syntax-error-1.js b/test/fixtures/es-modules/input-type-auto-scope/syntax-error-1.js similarity index 100% rename from test/fixtures/es-modules/entry-type-auto-scope/syntax-error-1.js rename to test/fixtures/es-modules/input-type-auto-scope/syntax-error-1.js diff --git a/test/fixtures/es-modules/entry-type-auto-scope/syntax-error-2.js b/test/fixtures/es-modules/input-type-auto-scope/syntax-error-2.js similarity index 100% rename from test/fixtures/es-modules/entry-type-auto-scope/syntax-error-2.js rename to test/fixtures/es-modules/input-type-auto-scope/syntax-error-2.js From aff36867b77145068f3422b61fa5841c0675b838 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 4 Apr 2019 00:05:45 -0700 Subject: [PATCH 12/13] esm: make --input-type=auto tests use string input --- test/es-module/test-esm-input-type-auto.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/test/es-module/test-esm-input-type-auto.js b/test/es-module/test-esm-input-type-auto.js index f9eaaaf3f6..b6d5136793 100644 --- a/test/es-module/test-esm-input-type-auto.js +++ b/test/es-module/test-esm-input-type-auto.js @@ -2,29 +2,35 @@ const common = require('../common'); const assert = require('assert'); const exec = require('child_process').execFile; +const { readFileSync } = require('fs'); const version = process.version; expect('esm-with-import-statement.js', version); expect('esm-with-export-statement.js', version); -expect('esm-with-import-expression.js', version); +// TODO: Enable import expression tests once import() is supported +// in string input. +// expect('esm-with-import-expression.js', version); expect('esm-with-indented-import-statement.js', version); expect('cjs-with-require.js', version); -expect('cjs-with-import-expression.js', version); +// expect('cjs-with-import-expression.js', version); expect('cjs-with-property-named-import.js', version); expect('cjs-with-property-named-export.js', version); expect('cjs-with-string-containing-import.js', version); expect('print-version.js', version); -expect('ambiguous-with-import-expression.js', version); +// expect('ambiguous-with-import-expression.js', version); expect('syntax-error-1.js', 'SyntaxError', true); expect('syntax-error-2.js', 'SyntaxError', true); function expect(file, want, wantsError = false) { const argv = [ - require.resolve(`../fixtures/es-modules/input-type-auto-scope/${file}`) + '--eval', + readFileSync( + require.resolve(`../fixtures/es-modules/input-type-auto-scope/${file}`), + 'utf8') ]; const opts = { // TODO: Remove when --experimental-modules is unflagged From c0eeba42bd5b34dcd57e396b56cd8dbb0c7ff168 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 4 Apr 2019 00:23:15 -0700 Subject: [PATCH 13/13] esm: fixup: --input-type doesn't apply to files --- lib/internal/modules/esm/default_resolve.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index 87025d67c8..a83cf9c675 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -3,7 +3,7 @@ const internalFS = require('internal/fs/utils'); const { NativeModule } = require('internal/bootstrap/loaders'); const { extname } = require('path'); -const { readFileSync, realpathSync } = require('fs'); +const { realpathSync } = require('fs'); const { getOptionValue } = require('internal/options'); const preserveSymlinks = getOptionValue('--preserve-symlinks'); @@ -94,12 +94,6 @@ function resolve(specifier, parentURL) { // entry point, etc.). throw new ERR_INPUT_TYPE_NOT_ALLOWED(); } - // --input-type=auto detects an ESM .js file within a CommonJS scope. - if (isMain && format === 'commonjs' && typeFlag === 'auto') { - const filename = fileURLToPath(url); - const source = readFileSync(filename, 'utf8'); - format = require('internal/modules/esm/detect_type')(source, filename); - } if (!format) { if (isMain) format = type === TYPE_MODULE ? 'module' : 'commonjs';