-
Notifications
You must be signed in to change notification settings - Fork 30k
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.
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
repl: display dynamic import version in error message. #48129
Conversation
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
).
@aduh95 I ran
update: ha! will fix it.
Meanwhile, any idea, why this could happen? Looks like a parse error?
|
|
On
^ That should be because it doesn't even parse till |
@aduh95 How come the test passes for: {
send: 'import "foo";',
expect: ''
}
|
@aduh95 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)
|
That's unlikely, we use npm to download new versions. node/tools/dep_updaters/update-acorn.sh Line 36 in 300f68e
|
acron#L9 reads Also, everything works fine in runkit gist. |
Ah, indeed it was
RE is not being respected? |
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'); ```
Build says: Also, finally looks like we have all the variants covered: |
If you push a commit to get rid of all the unrelated space changes, that should trigger a new CI run |
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: ***@***.***>
|
That's unlikely, but anyway, you would need to get rid of the unrelated changes to move this forward. |
Co-authored-by: Jordan Harband <ljharb@gmail.com>
Co-authored-by: Jordan Harband <ljharb@gmail.com>
Landed in 0c875bb |
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 <duhamelantoine1995@gmail.com>
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 <duhamelantoine1995@gmail.com>
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 <duhamelantoine1995@gmail.com>
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 <duhamelantoine1995@gmail.com>
hi @hemanth this commit is breaking tests on |
Hey @ruyadorno Ill try to backport it, thanks for highlighting! |
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 <duhamelantoine1995@gmail.com>
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 <duhamelantoine1995@gmail.com>
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 <duhamelantoine1995@gmail.com>
Fix for #48084