Skip to content

repl: display dynamic import version in error message. #48129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nodejs-github-bot merged 33 commits into from
Jun 11, 2023
Merged

repl: display dynamic import version in error message. #48129

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
3bb9b4e
repl: display dynamic import variant in static import error messages
hemanth May 27, 2023
653dd2b
fix: handle edge cases for toDynamicImport
hemanth May 31, 2023
65aff1f
use primordials
hemanth May 31, 2023
d2891e1
__proto__ to null
hemanth May 31, 2023
0752958
use primordials
hemanth May 31, 2023
1837614
use primordials
hemanth May 31, 2023
16ea5fc
use primordials
hemanth May 31, 2023
b1b6a8a
fix: quotes in test
hemanth May 31, 2023
3f08ac7
lint: space fix
hemanth May 31, 2023
3b569cf
feat: remove const declaration
hemanth Jun 1, 2023
708665d
fix: whitespaces
hemanth Jun 1, 2023
6e2f835
Update test/parallel/test-repl.js
hemanth Jun 1, 2023
74df404
Update test/parallel/test-repl.js
hemanth Jun 1, 2023
f259a5f
Update test/parallel/test-repl.js
hemanth Jun 1, 2023
556fa83
Update test/parallel/test-repl.js
hemanth Jun 1, 2023
f42f73c
Update test/parallel/test-repl.js
hemanth Jun 1, 2023
a83915d
Update lib/repl.js
hemanth Jun 1, 2023
64842ac
Update lib/repl.js
hemanth Jun 1, 2023
c7c8488
Update test/parallel/test-repl.js
hemanth Jun 1, 2023
00348c9
Update lib/repl.js
hemanth Jun 1, 2023
7d4b2c9
fix: missing imports
hemanth Jun 1, 2023
01daa46
fixup! fix: missing imports
aduh95 Jun 1, 2023
d59d33d
Update test/parallel/test-repl.js
hemanth Jun 1, 2023
91210be
test: fix deafult import
hemanth Jun 2, 2023
4d61565
fix: quote fix
hemanth Jun 2, 2023
9a619e5
Update lib/repl.js
hemanth Jun 2, 2023
9ef0490
test: fix quotes
hemanth Jun 2, 2023
605c8cb
Update lib/repl.js
hemanth Jun 2, 2023
89e992c
Update lib/repl.js
hemanth Jun 3, 2023
1b0bb6c
fix: specifier index check
hemanth Jun 10, 2023
b9903ba
chore: typo specifiers
hemanth Jun 10, 2023
e9fe193
Update lib/repl.js
hemanth Jun 10, 2023
c045901
chore: sort imports
hemanth Jun 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 71 additions & 42 deletions lib/repl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const {
ArrayPrototypeSome,
ArrayPrototypeSort,
ArrayPrototypeSplice,
ArrayPrototypeToString,
ArrayPrototypeUnshift,
Boolean,
Error,
Expand Down Expand Up @@ -104,7 +105,12 @@ const {
const {
isIdentifierStart,
isIdentifierChar,
parse: acornParse,
} = require('internal/deps/acorn/acorn/dist/acorn');


const acornWalk = require('internal/deps/acorn/acorn-walk/dist/walk');

const {
decorateErrorStack,
isError,
Expand Down Expand Up @@ -223,6 +229,29 @@ module.paths = CJSModule._nodeModulePaths(module.filename);
const writer = (obj) => inspect(obj, writer.options);
writer.options = { ...inspect.defaultOptions, showProxy: true };

// Converts static import statement to dynamic import statement
const toDynamicImport = (codeLine) => {
let dynamicImportStatement = '';
let moduleName = '';
const toCamelCase = (str) => str.replace(/[-_](\w)/g, (_, c) => c.toUpperCase());
const ast = acornParse(codeLine, { sourceType: 'module', ecmaVersion: 'latest' });
acornWalk.ancestor(ast, {
ImportDeclaration: (node) => {
const importedModules = node.source.value;
const importedSpecifiers = node.specifiers.map((specifier) => specifier.local.name);
if (importedSpecifiers.length > 1) {
moduleName = `{${importedSpecifiers.join(',')}}`;
} else {
const formattedSpecifiers = importedSpecifiers.length ? ArrayPrototypeToString(importedSpecifiers) : '';
moduleName = toCamelCase(formattedSpecifiers || importedModules);
}
dynamicImportStatement += `const ${moduleName} = await import('${importedModules}');`;
},
});
return dynamicImportStatement;
};


function REPLServer(prompt,
stream,
eval_,
Expand Down Expand Up @@ -283,13 +312,13 @@ function REPLServer(prompt,
get: pendingDeprecation ?
deprecate(() => this.input,
'repl.inputStream and repl.outputStream are deprecated. ' +
'Use repl.input and repl.output instead',
'Use repl.input and repl.output instead',
'DEP0141') :
() => this.input,
set: pendingDeprecation ?
deprecate((val) => this.input = val,
'repl.inputStream and repl.outputStream are deprecated. ' +
'Use repl.input and repl.output instead',
'Use repl.input and repl.output instead',
'DEP0141') :
(val) => this.input = val,
enumerable: false,
Expand All @@ -300,13 +329,13 @@ function REPLServer(prompt,
get: pendingDeprecation ?
deprecate(() => this.output,
'repl.inputStream and repl.outputStream are deprecated. ' +
'Use repl.input and repl.output instead',
'Use repl.input and repl.output instead',
'DEP0141') :
() => this.output,
set: pendingDeprecation ?
deprecate((val) => this.output = val,
'repl.inputStream and repl.outputStream are deprecated. ' +
'Use repl.input and repl.output instead',
'Use repl.input and repl.output instead',
'DEP0141') :
(val) => this.output = val,
enumerable: false,
Expand Down Expand Up @@ -344,9 +373,9 @@ function REPLServer(prompt,
// instance and that could trigger the `MaxListenersExceededWarning`.
process.prependListener('newListener', (event, listener) => {
if (event === 'uncaughtException' &&
process.domain &&
listener.name !== 'domainUncaughtExceptionClear' &&
domainSet.has(process.domain)) {
process.domain &&
listener.name !== 'domainUncaughtExceptionClear' &&
domainSet.has(process.domain)) {
// Throw an error so that the event will not be added and the current
// domain takes over. That way the user is notified about the error
// and the current code evaluation is stopped, just as any other code
Expand All @@ -363,8 +392,8 @@ function REPLServer(prompt,
const savedRegExMatches = ['', '', '', '', '', '', '', '', '', ''];
const sep = '\u0000\u0000\u0000';
const regExMatcher = new RegExp(`^${sep}(.*)${sep}(.*)${sep}(.*)${sep}(.*)` +
`${sep}(.*)${sep}(.*)${sep}(.*)${sep}(.*)` +
`${sep}(.*)$`);
`${sep}(.*)${sep}(.*)${sep}(.*)${sep}(.*)` +
`${sep}(.*)$`);

eval_ = eval_ || defaultEval;

Expand Down Expand Up @@ -417,7 +446,7 @@ function REPLServer(prompt,
// an expression. Note that if the above condition changes,
// lib/internal/repl/utils.js needs to be changed to match.
if (RegExpPrototypeExec(/^\s*{/, code) !== null &&
RegExpPrototypeExec(/;\s*$/, code) === null) {
RegExpPrototypeExec(/;\s*$/, code) === null) {
code = `(${StringPrototypeTrim(code)})\n`;
wrappedCmd = true;
}
Expand Down Expand Up @@ -492,7 +521,7 @@ function REPLServer(prompt,
while (true) {
try {
if (self.replMode === module.exports.REPL_MODE_STRICT &&
RegExpPrototypeExec(/^\s*$/, code) === null) {
RegExpPrototypeExec(/^\s*$/, code) === null) {
// "void 0" keeps the repl from returning "use strict" as the result
// value for statements and declarations that don't return a value.
code = `'use strict'; void 0;\n${code}`;
Expand Down Expand Up @@ -684,7 +713,7 @@ function REPLServer(prompt,
'module';
if (StringPrototypeIncludes(e.message, importErrorStr)) {
e.message = 'Cannot use import statement inside the Node.js ' +
'REPL, alternatively use dynamic import';
'REPL, alternatively use dynamic import: ' + toDynamicImport(self.lines.at(-1));
e.stack = SideEffectFreeRegExpPrototypeSymbolReplace(
/SyntaxError:.*\n/,
e.stack,
Expand Down Expand Up @@ -712,7 +741,7 @@ function REPLServer(prompt,
}

if (options[kStandaloneREPL] &&
process.listenerCount('uncaughtException') !== 0) {
process.listenerCount('uncaughtException') !== 0) {
process.nextTick(() => {
process.emit('uncaughtException', e);
self.clearBufferedCommand();
Expand All @@ -729,7 +758,7 @@ function REPLServer(prompt,
errStack = '';
ArrayPrototypeForEach(lines, (line) => {
if (!matched &&
RegExpPrototypeExec(/^\[?([A-Z][a-z0-9_]*)*Error/, line) !== null) {
RegExpPrototypeExec(/^\[?([A-Z][a-z0-9_]*)*Error/, line) !== null) {
errStack += writer.options.breakLength >= line.length ?
`Uncaught ${line}` :
`Uncaught:\n${line}`;
Expand Down Expand Up @@ -875,8 +904,8 @@ function REPLServer(prompt,
// display next prompt and return.
if (trimmedCmd) {
if (StringPrototypeCharAt(trimmedCmd, 0) === '.' &&
StringPrototypeCharAt(trimmedCmd, 1) !== '.' &&
NumberIsNaN(NumberParseFloat(trimmedCmd))) {
StringPrototypeCharAt(trimmedCmd, 1) !== '.' &&
NumberIsNaN(NumberParseFloat(trimmedCmd))) {
const matches = RegExpPrototypeExec(/^\.([^\s]+)\s*(.*)$/, trimmedCmd);
const keyword = matches && matches[1];
const rest = matches && matches[2];
Expand All @@ -901,10 +930,10 @@ function REPLServer(prompt,
ReflectApply(_memory, self, [cmd]);

if (e && !self[kBufferedCommandSymbol] &&
StringPrototypeStartsWith(StringPrototypeTrim(cmd), 'npm ')) {
StringPrototypeStartsWith(StringPrototypeTrim(cmd), 'npm ')) {
self.output.write('npm should be run outside of the ' +
'Node.js REPL, in your normal shell.\n' +
'(Press Ctrl+D to exit.)\n');
'Node.js REPL, in your normal shell.\n' +
'(Press Ctrl+D to exit.)\n');
self.displayPrompt();
return;
}
Expand All @@ -929,11 +958,11 @@ function REPLServer(prompt,

// If we got any output - print it (if no error)
if (!e &&
// When an invalid REPL command is used, error message is printed
// immediately. We don't have to print anything else. So, only when
// the second argument to this function is there, print it.
arguments.length === 2 &&
(!self.ignoreUndefined || ret !== undefined)) {
// When an invalid REPL command is used, error message is printed
// immediately. We don't have to print anything else. So, only when
// the second argument to this function is there, print it.
arguments.length === 2 &&
(!self.ignoreUndefined || ret !== undefined)) {
if (!self.underscoreAssigned) {
self.last = ret;
}
Expand Down Expand Up @@ -984,7 +1013,7 @@ function REPLServer(prompt,
if (!self.editorMode || !self.terminal) {
// Before exiting, make sure to clear the line.
if (key.ctrl && key.name === 'd' &&
self.cursor === 0 && self.line.length === 0) {
self.cursor === 0 && self.line.length === 0) {
self.clearLine();
}
clearPreview(key);
Expand Down Expand Up @@ -1181,7 +1210,7 @@ const importRE = /\bimport\s*\(\s*['"`](([\w@./:-]+\/)?(?:[\w@./:-]*))(?![^'"`])
const requireRE = /\brequire\s*\(\s*['"`](([\w@./:-]+\/)?(?:[\w@./:-]*))(?![^'"`])$/;
const fsAutoCompleteRE = /fs(?:\.promises)?\.\s*[a-z][a-zA-Z]+\(\s*["'](.*)/;
const simpleExpressionRE =
/(?:[\w$'"`[{(](?:\w|\$|['"`\]})])*\??\.)*[a-zA-Z_$](?:\w|\$)*\??\.?$/;
/(?:[\w$'"`[{(](?:\w|\$|['"`\]})])*\??\.)*[a-zA-Z_$](?:\w|\$)*\??\.?$/;
const versionedFileNamesRe = /-\d+\.\d+/;

function isIdentifier(str) {
Expand Down Expand Up @@ -1337,15 +1366,15 @@ function complete(line, callback) {
const dirents = gracefulReaddir(dir, { withFileTypes: true }) || [];
ArrayPrototypeForEach(dirents, (dirent) => {
if (RegExpPrototypeExec(versionedFileNamesRe, dirent.name) !== null ||
dirent.name === '.npm') {
dirent.name === '.npm') {
// Exclude versioned names that 'npm' installs.
return;
}
const extension = path.extname(dirent.name);
const base = StringPrototypeSlice(dirent.name, 0, -extension.length);
if (!dirent.isDirectory()) {
if (StringPrototypeIncludes(extensions, extension) &&
(!subdir || base !== 'index')) {
(!subdir || base !== 'index')) {
ArrayPrototypePush(group, `${subdir}${base}`);
}
return;
Expand Down Expand Up @@ -1398,7 +1427,7 @@ function complete(line, callback) {
ArrayPrototypeForEach(dirents, (dirent) => {
const { name } = dirent;
if (RegExpPrototypeExec(versionedFileNamesRe, name) !== null ||
name === '.npm') {
name === '.npm') {
// Exclude versioned names that 'npm' installs.
return;
}
Expand Down Expand Up @@ -1431,20 +1460,20 @@ function complete(line, callback) {

ArrayPrototypePush(completionGroups, _builtinLibs, nodeSchemeBuiltinLibs);
} else if ((match = RegExpPrototypeExec(fsAutoCompleteRE, line)) !== null &&
this.allowBlockingCompletions) {
this.allowBlockingCompletions) {
({ 0: completionGroups, 1: completeOn } = completeFSFunctions(match));
// Handle variable member lookup.
// We support simple chained expressions like the following (no function
// calls, etc.). That is for simplicity and also because we *eval* that
// leading expression so for safety (see WARNING above) don't want to
// eval function calls.
//
// foo.bar<|> # completions for 'foo' with filter 'bar'
// spam.eggs.<|> # completions for 'spam.eggs' with filter ''
// foo<|> # all scope vars with filter 'foo'
// foo.<|> # completions for 'foo' with filter ''
// Handle variable member lookup.
// We support simple chained expressions like the following (no function
// calls, etc.). That is for simplicity and also because we *eval* that
// leading expression so for safety (see WARNING above) don't want to
// eval function calls.
//
// foo.bar<|> # completions for 'foo' with filter 'bar'
// spam.eggs.<|> # completions for 'spam.eggs' with filter ''
// foo<|> # all scope vars with filter 'foo'
// foo.<|> # completions for 'foo' with filter ''
} else if (line.length === 0 ||
RegExpPrototypeExec(/\w|\.|\$/, line[line.length - 1]) !== null) {
RegExpPrototypeExec(/\w|\.|\$/, line[line.length - 1]) !== null) {
const { 0: match } = RegExpPrototypeExec(simpleExpressionRE, line) || [''];
if (line.length !== 0 && !match) {
completionGroupsLoaded();
Expand Down Expand Up @@ -1495,7 +1524,7 @@ function complete(line, callback) {
try {
let p;
if ((typeof obj === 'object' && obj !== null) ||
typeof obj === 'function') {
typeof obj === 'function') {
memberGroups.push(filteredOwnPropertyNames(obj));
p = ObjectGetPrototypeOf(obj);
} else {
Expand Down
36 changes: 35 additions & 1 deletion test/parallel/test-repl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,41 @@ const tcpTests = [
kArrow,
'',
'Uncaught:',
/^SyntaxError: .* dynamic import/,
'SyntaxError: Cannot use import statement inside the Node.js REPL, \
alternatively use dynamic import: const comeOn = await import(\'fhqwhgads\');',
]
},
{
send: 'import { export1, export2 } from "module-name"',
expect: [
kSource,
kArrow,
'',
'Uncaught:',
'SyntaxError: Cannot use import statement inside the Node.js REPL, \
alternatively use dynamic import: const {export1,export2} = await import(\'module-name\');',
]
},
{
send: 'import * as name from "module-name";',
expect: [
kSource,
kArrow,
'',
'Uncaught:',
'SyntaxError: Cannot use import statement inside the Node.js REPL, \
alternatively use dynamic import: const name = await import(\'module-name\');',
]
},
{
send: 'import "module-name";',
expect: [
kSource,
kArrow,
'',
'Uncaught:',
'SyntaxError: Cannot use import statement inside the Node.js REPL, \
alternatively use dynamic import: const moduleName = await import(\'module-name\');',
Copy link
Contributor

@aduh95 aduh95 May 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
alternatively use dynamic import: const moduleName = await import(\'module-name\');',
alternatively use dynamic import: await import(\'module-name\');',

Copy link
Contributor Author

@hemanth hemanth Jun 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we just wanted this, we need no acron and likes? We could have just extracted the label after from and wrap it in an await import(...)?

Copy link
Member

@ljharb ljharb Jun 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it depends, i think. if i static import with explicit bindings, or with * as, then those need to be used to set the result of await import().

Copy link
Contributor Author

@hemanth hemanth Jun 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the error message if all that we need is await import(<module_name>) it can be a RE?

Copy link
Member

@ljharb ljharb Jun 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to the complexity of import statements and their grammar, I don't think anything short of a parser will be a reliable approach.

If using a parser isn't desirable, then I'm not sure why we'd even bother with more than a link to MDN's page on dynamic import, and they can figure it out for themselves.

Copy link
Contributor

@aduh95 aduh95 Jun 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the user wrote import 'module-name' in REPL, they meant await import("module-name"), not const moduleName = await import('module-name') (otherwise they would have written import * as moduleName from 'module-name')). Using Acorn is probably still more reliable, and more future proof.

]
},
];
Expand Down