Skip to content
This repository has been archived by the owner on Apr 16, 2020. It is now read-only.

Commit

Permalink
esm: --type=auto should throw syntax errors directly
Browse files Browse the repository at this point in the history
  • Loading branch information
GeoffreyBooth committed Mar 9, 2019
1 parent a227deb commit 4d841b3
Show file tree
Hide file tree
Showing 9 changed files with 31 additions and 9 deletions.
3 changes: 2 additions & 1 deletion lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,8 @@ E('ERR_INVALID_SYNC_FORK_INPUT',
E('ERR_INVALID_THIS', 'Value of "this" must be of type %s', TypeError);
E('ERR_INVALID_TUPLE', '%s must be an iterable %s tuple', TypeError);
E('ERR_INVALID_TYPE_FLAG',
'Type flag must be one of "auto", "commonjs", or "module". Received --type=%s',
'Type flag must be one of "auto", "commonjs", or "module". ' +
'Received --type=%s',
TypeError);
E('ERR_INVALID_URI', 'URI malformed', URIError);
E('ERR_INVALID_URL', 'Invalid URL: %s', TypeError);
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/main/check_syntax.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ function checkSyntax(source, filename) {
if (filename === '[stdin]' || filename === '[eval]') {
const { typeFlag } = require('internal/process/esm_loader');
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());
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/main/eval_stdin.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ readStdin((code) => {
process._eval = code;
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);
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/main/eval_string.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ addBuiltinLibsToObject(global);
markBootstrapComplete();
if (typeFlag === 'module' ||
(typeFlag === 'auto' &&
require('internal/modules/esm/detect_type')(source) === 'module'))
require('internal/modules/esm/detect_type')(source, '[eval]') === 'module'))
evalModule(source);
else
evalScript('[eval]', source, process._breakFirstLine);
5 changes: 3 additions & 2 deletions lib/internal/modules/esm/default_resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,9 @@ function getModuleFormat(url, isMain, parentURL) {

// --type=auto detects an ESM .js file within a CommonJS scope.
if (isMain && format === 'commonjs' && asyncESM.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);
}

return format;
Expand Down
12 changes: 10 additions & 2 deletions lib/internal/modules/esm/detect_type.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)) {
Expand All @@ -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';
}
Expand Down
10 changes: 9 additions & 1 deletion test/es-module/test-esm-type-auto.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}`)
];
Expand All @@ -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>`);
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/es-modules/type-auto-scope/syntax-error-1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export
2 changes: 2 additions & 0 deletions test/fixtures/es-modules/type-auto-scope/syntax-error-2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const str = 'import
var foo = 3;

0 comments on commit 4d841b3

Please sign in to comment.