GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — repl: display dynamic import version in error message. by hemanth · Pull Request #48129 · nodejs/node

Conversation

@hemanth

@hemanth hemanth commented May 22, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Fix for #48084

❯ ./out/Release/node
Welcome to Node.js v21.0.0-pre.
Type ".help" for more information.
> import {foo, bar} from 'mo';
import {foo, bar} from 'mo';
^^^^^^

Uncaught:
SyntaxError: Cannot use import statement inside the Node.js REPL, alternatively use dynamic import: const {foo,bar} = await import('mo');
image

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss and thoughtsunificator reacted with thumbs up emoji atlowChemi reacted with rocket emoji
All reactions
  • 👍 2 reactions
  • 🚀 1 reaction
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. repl Issues and PRs related to the REPL subsystem. labels May 22, 2023
@hemanth
hemanth force-pushed the main branch from 4f9b1e8 to 84ca0b2 Compare May 23, 2023 05:11
aduh95
aduh95 reviewed May 23, 2023
View reviewed changes

@aduh95 aduh95 left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Can you please remove the unrelated spacing changes, and ensure we have a test for all types of static import statements as well as tests for when there are more than one statement (e.g. const a=1;import 'url';const b=2).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@hemanth

hemanth commented May 23, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

@aduh95 I ran make lint-js-fix, looks better now?

const a=1;import 'url';const b=2 that's a good catch, pretty sure that would bomb, will have a look.

update: ha! will fix it.

>const a=1;import 'url';const b=2
const a=1;import 'url';const b=2
          ^^^^^^

Uncaught:
SyntaxError: Cannot use import statement inside the Node.js REPL, alternatively use dynamic import: const  = await import('url');

Meanwhile, any idea, why this could happen? Looks like a parse error?

> import comeOn from \'fhqwhgads\'
Uncaught [SyntaxError: Expecting Unicode escape sequence \uXXXX (1:20)
] {
  pos: 20,
  loc: Position { line: 1, column: 20 },
  raisedAt: 20
}
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aduh95

aduh95 commented May 23, 2023

Copy link
Copy Markdown
Contributor

import comeOn from \'fhqwhgads\' is not valid ECMAScript (mind the backslashes), so it's expected that it produces a parse error, or am I missing something?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hemanth

hemanth commented May 23, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

On v20.1.0 we see:

❯ node
Welcome to Node.js v20.1.0.
Type ".help" for more information.
> import comeOn from \'fhqwhgads\'
import comeOn from \'fhqwhgads\'
^^^^^^

Uncaught:
SyntaxError: Cannot use import statement inside the Node.js REPL, alternatively use dynamic import

^ That should be because it doesn't even parse till \'fhqwhgads\' so the expected behavior with the new messaging would be?
aduh95 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hemanth

hemanth commented May 23, 2023

Copy link
Copy Markdown
Contributor Author

Works in runkit, because it interpolates the string well?

image
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hemanth
hemanth force-pushed the main branch from 84ca0b2 to cd2c691 Compare May 24, 2023 23:06
@hemanth

hemanth commented May 25, 2023

Copy link
Copy Markdown
Contributor Author

@aduh95 How come the test passes for:

{
  send: 'import "foo";',
  expect: ''
}
❯ tools/test.py test/parallel/test-repl-*
[00:03|% 100|+  72|-   0]: Done                                               

All tests passed.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hemanth

hemanth commented May 27, 2023

Copy link
Copy Markdown
Contributor Author

@aduh95 internal/deps/acorn/acorn/dist/acorn and internal/deps/acorn/acorn-walk/dist/walk have different implementations compared what is on npm.

Currently stuck with:

Uncaught TypeError: baseVisitor[type] is not a function
    at c (node:internal/deps/acorn/acorn-walk/dist/walk:45:24)
    at Object.ancestor (node:internal/deps/acorn/acorn-walk/dist/walk:48:7)
    at toDynamicImport (node:repl:238:13)
    at Domain.debugDomainError (node:repl:713:62)
    at Domain.emit (node:events:511:28)
    at Domain.emit (node:domain:489:12)
    at finish (node:repl:949:22)
    at finishExecution (node:repl:571:7)
    at REPLServer.defaultEval (node:repl:660:7)
    at bound (node:domain:433:15)

Parser(options, input, startPos) param positions are different...
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aduh95

aduh95 commented May 27, 2023

Copy link
Copy Markdown
Contributor

That's unlikely, we use npm to download new versions.

node/tools/dep_updaters/update-acorn.sh

Line 36 in 300f68e

"$NODE" "$NPM" install --global-style --no-bin-links --ignore-scripts "acorn@$NEW_VERSION"

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hemanth

hemanth commented May 27, 2023

Copy link
Copy Markdown
Contributor Author

acron#L9 reads constructor(options, input, startPos) their readme interface reads parse(input, options) ha, maybe it is parse vs Parser.

Also, everything works fine in runkit gist.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hemanth

hemanth commented May 27, 2023

Copy link
Copy Markdown
Contributor Author

Ah, indeed it was Parser vs parse.

  code: 'ERR_ASSERTION',
  actual: "SyntaxError: Cannot use import statement inside the Node.js REPL, alternatively use dynamic import: const comeOn = await import('fhqwhgads');",
  expected: /^SyntaxError: .* dynamic import: const comeOn = await import('fhqwhgads');/,
  operator: 'match'

RE is not being respected? /^SyntaxError: .* dynamic import:..
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hemanth hemanth closed this May 27, 2023
@hemanth
hemanth force-pushed the main branch from cd2c691 to d2a1f71 Compare May 27, 2023 14:14
@hemanth hemanth reopened this May 27, 2023
@hemanth
repl: display dynamic import variant in static import error messages
3bb9b4e 
Enhance the REPL message for static import error message.

```
> import {foo, bar} from 'moo';
import {foo, bar} from 'moo';
^^^^^^

Uncaught:
SyntaxError: .* dynamic import: const {foo,bar} = await import('moo');
```
@hemanth
hemanth force-pushed the main branch from f7620bd to 3bb9b4e Compare May 27, 2023 14:27
@hemanth

hemanth commented May 27, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Build says: Error: The operation was canceled. @aduh95 is there a keyword to re-trigger CI?

image

Also, finally looks like we have all the variants covered:

image
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aduh95

aduh95 commented May 27, 2023

Copy link
Copy Markdown
Contributor

If you push a commit to get rid of all the unrelated space changes, that should trigger a new CI run
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hemanth

hemanth commented May 27, 2023 via email

Copy link
Copy Markdown
Contributor Author
Ha! The spaces changes came in from lint-fix I presume.
…
On Sat, May 27, 2023, 8:09 AM Antoine du Hamel ***@***.***> wrote: If you push a commit to get rid of all the unrelated space changes, that should trigger a new CI run — Reply to this email directly, view it on GitHub <#48129 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAEPC2G3CGRJY4ACLMNOO3XIIKKLANCNFSM6AAAAAAYLC4TYY> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aduh95

aduh95 commented May 27, 2023

Copy link
Copy Markdown
Contributor

The spaces changes came in from lint-fix I presume.

That's unlikely, but anyway, you would need to get rid of the unrelated changes to move this forward.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 reviewed May 27, 2023
View reviewed changes
Comment thread lib/repl.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lib/repl.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread test/parallel/test-repl.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@hemanth
fix: handle edge cases for toDynamicImport
653dd2b
@hemanth
hemanth force-pushed the main branch from c9a4d6b to 653dd2b Compare May 31, 2023 04:39
hemanth
hemanth commented May 31, 2023
View reviewed changes
Comment thread lib/repl.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
ljharb
ljharb reviewed May 31, 2023
View reviewed changes
Comment thread lib/repl.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lib/repl.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lib/repl.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lib/repl.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lib/repl.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
hemanth and others added 2 commits May 31, 2023 12:23
@hemanth @ljharb
use primordials
65aff1f 
Co-authored-by: Jordan Harband <[email protected]>
@hemanth @ljharb
__proto__ to null
d2891e1 
Co-authored-by: Jordan Harband <[email protected]>
@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Jun 10, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Jun 10, 2023
@nodejs-github-bot

nodejs-github-bot commented Jun 10, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52190/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Jun 10, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52200/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot mentioned this pull request Jun 11, 2023
CI Reliability 2023-06-11 nodejs/reliability#590
Open
25 tasks
@aduh95 aduh95 added commit-queue PRs queued for automated landing through the Commit Queue. commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels Jun 11, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Jun 11, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 0c875bb into nodejs:main Jun 11, 2023
@nodejs-github-bot

nodejs-github-bot commented Jun 11, 2023

Copy link
Copy Markdown
Collaborator

Landed in 0c875bb
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hemanth

hemanth commented Jun 11, 2023

Copy link
Copy Markdown
Contributor Author

🥳 Thank you @aduh95 and @ljharb 🤓
aduh95 and Mesteery reacted with heart emoji
All reactions
  • ❤️ 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
This was referenced Jun 12, 2023
CI Reliability 2023-06-12 nodejs/reliability#591
Open
CI Reliability 2023-06-13 nodejs/reliability#592
Open
CI Reliability 2023-06-14 nodejs/reliability#593
Open
CI Reliability 2023-06-15 nodejs/reliability#594
Open
RafaelGSS pushed a commit that referenced this pull request Jul 3, 2023
@hemanth @RafaelGSS
repl: display dynamic import variant in static import error messages
84fe811 
Enhance the REPL message for static import error message.

```
> import {foo, bar} from 'moo';
import {foo, bar} from 'moo';
^^^^^^

Uncaught:
SyntaxError: .* dynamic import: const {foo,bar} = await import('moo');
```

PR-URL: #48129
Reviewed-By: Antoine du Hamel <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Jul 3, 2023
v20.4.0 proposal #48643
Merged
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@hemanth @Ceres6
repl: display dynamic import variant in static import error messages
65a0b58 
Enhance the REPL message for static import error message.

```
> import {foo, bar} from 'moo';
import {foo, bar} from 'moo';
^^^^^^

Uncaught:
SyntaxError: .* dynamic import: const {foo,bar} = await import('moo');
```

PR-URL: nodejs#48129
Reviewed-By: Antoine du Hamel <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@hemanth @Ceres6
repl: display dynamic import variant in static import error messages
9710666 
Enhance the REPL message for static import error message.

```
> import {foo, bar} from 'moo';
import {foo, bar} from 'moo';
^^^^^^

Uncaught:
SyntaxError: .* dynamic import: const {foo,bar} = await import('moo');
```

PR-URL: nodejs#48129
Reviewed-By: Antoine du Hamel <[email protected]>
ruyadorno pushed a commit that referenced this pull request Aug 29, 2023
@hemanth @ruyadorno
repl: display dynamic import variant in static import error messages
22c4a9b 
Enhance the REPL message for static import error message.

```
> import {foo, bar} from 'moo';
import {foo, bar} from 'moo';
^^^^^^

Uncaught:
SyntaxError: .* dynamic import: const {foo,bar} = await import('moo');
```

PR-URL: #48129
Reviewed-By: Antoine du Hamel <[email protected]>
@ruyadorno ruyadorno mentioned this pull request Aug 29, 2023
v18.18.0 proposal #49220
Merged
@ruyadorno

ruyadorno commented Sep 1, 2023

Copy link
Copy Markdown
Member

hi @hemanth this commit is breaking tests on v18.x-staging branch and will need manual backport in order to land on v18.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ruyadorno ruyadorno added the backport-requested-v18.x label Sep 1, 2023
@hemanth

hemanth commented Sep 1, 2023

Copy link
Copy Markdown
Contributor Author

Hey @ruyadorno Ill try to backport it, thanks for highlighting!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
targos pushed a commit that referenced this pull request Nov 10, 2023
@hemanth @targos
repl: display dynamic import variant in static import error messages
bbafd42 
Enhance the REPL message for static import error message.

```
> import {foo, bar} from 'moo';
import {foo, bar} from 'moo';
^^^^^^

Uncaught:
SyntaxError: .* dynamic import: const {foo,bar} = await import('moo');
```

PR-URL: #48129
Reviewed-By: Antoine du Hamel <[email protected]>
@targos targos mentioned this pull request Nov 28, 2023
v18.19.0 release proposal #50953
Merged
@hemanth hemanth mentioned this pull request Mar 21, 2024
REPL: import statement inside the Node.js REPL #48084
Closed
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
@sercher
repl: display dynamic import variant in static import error
a24f67a 
Enhance the REPL message for static import error message.

```
> import {foo, bar} from 'moo';
import {foo, bar} from 'moo';
^^^^^^

Uncaught:
SyntaxError: .* dynamic import: const {foo,bar} = await import('moo');
```

PR-URL: nodejs/node#48129
Reviewed-By: Antoine du Hamel <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
@sercher
repl: display dynamic import variant in static import error
ba47e0e 
Enhance the REPL message for static import error message.

```
> import {foo, bar} from 'moo';
import {foo, bar} from 'moo';
^^^^^^

Uncaught:
SyntaxError: .* dynamic import: const {foo,bar} = await import('moo');
```

PR-URL: nodejs/node#48129
Reviewed-By: Antoine du Hamel <[email protected]>
@0xdevalias 0xdevalias mentioned this pull request May 28, 2025
REPL with package.json "type":"module" #33369
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ljharb ljharb ljharb left review comments
@aduh95 aduh95 aduh95 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue-squash PRs the Commit Queue should land as one squashed commit. needs-ci PRs that need a full CI run. repl Issues and PRs related to the REPL subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@hemanth @aduh95 @Mesteery @nodejs-github-bot @ruyadorno @ljharb
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.