repl: improve static import error message in repl #33588
Conversation
MylesBorins
added
the
esm
There was a problem hiding this comment.
The logic should probably be moved here:
Line 567 in 54b36e4
BridgeAR
force-pushed
the
master
branch
2 times, most recently
from
8ae28ff
to
2935f72
Compare
MylesBorins
force-pushed
the
better-repl-error
branch
from
94e46ab
to
576de38
Compare
|
I've updated PTAL @BridgeAR I don't think we need to check the length on this. If someone is explicitly messing with the stack size I'm pretty happy that this feature might fail for them. Thoughts? |
lib/repl.js
Outdated
| e.message = 'Cannot use import statement inside the Node.js repl, ' + | ||
| 'please use dynamic import instead'; | ||
| const splitStack = StringPrototypeSplit(e.stack, '\n'); | ||
| splitStack[4] = `SyntaxError: ${e.message}`; |
There was a problem hiding this comment.
we could alternative find which item in the array starts with SyntaxError rather than by default using the 5th item
There was a problem hiding this comment.
That sounds better to me. There is in fact similar code to determine that line below. It'll only be triggered in case there's no uncaughtException listener though.
There was a problem hiding this comment.
Updated to use regex PTAL
|
Is it not possible to just fix static import? That would be most user friendly and I would rather have that. I guess it might be tricky to implement it though? |
|
@BridgeAR AFAIK there is no way to implement static import in the REPL. I could be wrong, but in lieu of a solution to that this seems like the next best thing. |
Currently the error is rather ambiguous and does not inform folks that static import is not supported in the repl. This overrides the default error message with one that is more informative. Closes: nodejs#33576
MylesBorins
force-pushed
the
better-repl-error
branch
from
d83bb30
to
4a7fe9a
Compare
MylesBorins
added
the
author ready
|
hey @BridgeAR are you ok with this landing? |
|
Pinging @BridgeAR one more time. I'll land in 48 hours if I don't hear anything. |
Currently the error is rather ambiguous and does not inform folks that static import is not supported in the repl. This overrides the default error message with one that is more informative. Closes: #33576 PR-URL: #33588 Fixes: #33576 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
|
Landed in 861a54c |
| @@ -805,6 +805,16 @@ const tcpTests = [ | |||
| { | |||
| send: `require(${JSON.stringify(moduleFilename)}).number`, | |||
| expect: '42' | |||
| }, | |||
| { | |||
| send: 'import comeOn from \'fhqwhgads\'', | |||
There was a problem hiding this comment.
| send: 'import comeOn from \'fhqwhgads\'', | |
| send: 'import everybody from \'the limit\'', |
There was a problem hiding this comment.
On second thought...
| send: 'import comeOn from \'fhqwhgads\'', | |
| send: 'import theLimit from \'everybody\'', |
There was a problem hiding this comment.
Long descriptive names are preferable.
| send: 'import comeOn from \'fhqwhgads\'', | |
| send: 'import comeOn from \'fhqwhgadshgnsdhjsdbkhsdabkfabkveybvf\'', |
There was a problem hiding this comment.
LOLOLOL
guess this will need to come in a followup
Currently the error is rather ambiguous and does not inform folks that static import is not supported in the repl. This overrides the default error message with one that is more informative. Closes: #33576 PR-URL: #33588 Fixes: #33576 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Currently the error is rather ambiguous and does not inform folks that static import is not supported in the repl. This overrides the default error message with one that is more informative. Closes: #33576 PR-URL: #33588 Fixes: #33576 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Currently the error is rather ambiguous and does not inform folks that static import is not supported in the repl. This overrides the default error message with one that is more informative. Closes: #33576 PR-URL: #33588 Fixes: #33576 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Currently the error is rather ambiguous and does not inform folks that static import is not supported in the repl. This overrides the default error message with one that is more informative. Closes: #33576 PR-URL: #33588 Fixes: #33576 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Currently the error is rather ambiguous and does not inform folks that
static import is not supported in the repl. This overrides the default
error message with one that is more informative.
Closes: #33576