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 29 commits
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
28 changes: 27 additions & 1 deletion lib/repl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,11 @@ const {
ArrayPrototypeSort,
ArrayPrototypeSplice,
ArrayPrototypeUnshift,
ArrayPrototypeAt,
Copy link
Copy Markdown
Contributor

@aduh95 aduh95 Jun 2, 2023

Choose a reason for hiding this comment

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

Can you move this line between line 45 and line 46 please?

Copy link
Copy Markdown
Contributor Author

@hemanth hemanth Jun 2, 2023

Choose a reason for hiding this comment

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

Well, this PR is now blocked, until we have ArrayPrototypeAt or I just use the length attribute to fetch the last element.

Copy link
Copy Markdown
Contributor

@aduh95 aduh95 Jun 2, 2023

Choose a reason for hiding this comment

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

until we have ArrayPrototypeAt

What do you mean? We already have it.

Copy link
Copy Markdown
Contributor Author

@hemanth hemanth Jun 3, 2023

Choose a reason for hiding this comment

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

I don't find ArrayPrototypeAt in typings/primordials.d.ts

Copy link
Copy Markdown
Contributor

@aduh95 aduh95 Jun 9, 2023

Choose a reason for hiding this comment

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

I guess that means the typing file is outdated. That doesn't mean it's not there or that you can't use it though.

Copy link
Copy Markdown
Contributor

@aduh95 aduh95 Jun 9, 2023

Choose a reason for hiding this comment

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

Can you move this line between line 45 and line 46 please?

Copy link
Copy Markdown
Contributor Author

@hemanth hemanth Jun 10, 2023

Choose a reason for hiding this comment

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

@aduh95 Where else is it defined? Also, does the lint fix reorder the import statements?

Copy link
Copy Markdown
Contributor

@aduh95 aduh95 Jun 10, 2023

Choose a reason for hiding this comment

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

It's defined by V8

node/deps/v8/src/init/bootstrapper.cc

Lines 1792 to 1793 in f226350

SimpleInstallFunction(isolate_, proto, "at", Builtin::kArrayPrototypeAt, 1,
true);

Also, does the lint fix reorder the import statements?

No you need to do that manually.

Boolean,
Comment thread
hemanth marked this conversation as resolved.
Error,
FunctionPrototypeBind,
JSONStringify,
MathMaxApply,
NumberIsNaN,
NumberParseFloat,
Expand Down Expand Up @@ -104,7 +106,9 @@ 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 +227,28 @@ 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 = '';
const ast = acornParse(codeLine, { __proto__: null, sourceType: 'module', ecmaVersion: 'latest' });
acornWalk.ancestor(ast, {
ImportDeclaration(node) {
const awaitDynamicImport = `await import(${JSONStringify(node.source.value)});`;
if (node.specifiers.length === 0) {
dynamicImportStatement += awaitDynamicImport;
} else if (node.specifiers.length === 1 && node.specifiers[0].type === 'ImportNamespaceSpecifier') {
dynamicImportStatement += `const ${node.specifier[0].local.name} = ${awaitDynamicImport}`;
Comment thread
aduh95 marked this conversation as resolved.
Outdated
} else {
const importNames = ArrayPrototypeJoin(ArrayPrototypeMap(node.specifiers, ({ local, imported }) =>
(local.name === imported?.name ? local.name : `${imported?.name ?? 'default'}: ${local.name}`),
), ', ');
dynamicImportStatement += `const { ${importNames} } = ${awaitDynamicImport}`;
}
},
});
return dynamicImportStatement;
};

function REPLServer(prompt,
stream,
eval_,
Expand Down Expand Up @@ -684,7 +710,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(ArrayPrototypeAt(self.lines, -1));
e.stack = SideEffectFreeRegExpPrototypeSymbolReplace(
/SyntaxError:.*\n/,
e.stack,
Expand Down
69 changes: 68 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,74 @@ const tcpTests = [
kArrow,
'',
'Uncaught:',
/^SyntaxError: .* dynamic import/,
'SyntaxError: Cannot use import statement inside the Node.js REPL, \
alternatively use dynamic import: const { default: comeOn } = await import("fhqwhgads");',
]
},
{
send: 'import { export1, export2 } from "module-name"',
Comment thread
aduh95 marked this conversation as resolved.
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: await import("module-name");',
]
},
{
send: 'import { export1 as localName1, export2 } from "bar";',
expect: [
kSource,
kArrow,
'',
'Uncaught:',
'SyntaxError: Cannot use import statement inside the Node.js REPL, \
alternatively use dynamic import: const { export1: localName1, export2 } = await import("bar");',
]
},
{
send: 'import alias from "bar";',
expect: [
kSource,
kArrow,
'',
'Uncaught:',
'SyntaxError: Cannot use import statement inside the Node.js REPL, \
alternatively use dynamic import: const { default: alias } = await import("bar");',
]
},
{
send: 'import alias, {namedExport} from "bar";',
expect: [
kSource,
kArrow,
'',
'Uncaught:',
'SyntaxError: Cannot use import statement inside the Node.js REPL, \
alternatively use dynamic import: const { default: alias, namedExport } = await import("bar");',
]
},
];
Expand Down
⚡