repl: use inspector over vm#64034
Open
avivkeller wants to merge 2 commits into
Open
Conversation
Collaborator
|
Review requested:
|
9fdad96 to
56ebb41
Compare
3f34721 to
d7534b9
Compare
Member
Author
|
cc @nodejs/repl @nodejs/inspector |
Contributor
|
The
notable-change
Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section. |
Signed-off-by: Aviv Keller <me@aviv.sh>
412cc73 to
6c61672
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR switches the REPL from
node:vmtonode:inspector, moving it from asynchronous evaluator to an asynchronous one. This includes the following improvements:
--no-experimental-repl-awaitis no longer allowed, top-levelawaitis no longer experimental and is natively supported by the inspector. This
also resolves a known limitation in which TLA would invalidate the lexical
scoping of
constdeclarations.import ... from '...'are now transformed into realdynamic imports and executed, instead of throwing
Cannot use import statement outside a modulewith a hint to rewrite theimport manually. This is not strictly required by removing
node:vm, but itis made easier by the change. I can split this into a separate PR if
preferred.
node:vmissues are resolved (see the list below), and userscan now attach process listeners without breaking the REPL.
.load-ed file where an error was thrown.This is a semver-major change, see the reasons listed in the notable changes section.
Removed Tests
addons/repl-domain-abort,parallel/test-repl-domain.js: We no longer rely on thedomainmodule, so testing it is no longer needed.parallel/test-repl-no-terminal-restore-process-listeners.js: An extension of the above, we no longer modify the ability to set process listeners.parallel/test-repl-uncaught-exception-async.js/parallel/test-repl-uncaught-exception.js/parallel/test-repl-tab-complete-nested-repls.js: An extension of the above, this test case no longer throws an error.known_issues/test-repl-require-context.js: This known issue has been resolved.parallel/test-repl-top-level-await.js: There's no need to test TLA, it's supported natively by the inspector.parallel/test-repl-preprocess-top-level-await.js: An extension of the above, the REPL no longer pre-processes top level awaitparallel/test-repl-preview-without-inspector.js: The REPL no longer works without the inspectorparallel/test-util-sigint-watchdog.js: This tested code which has been removed as a part of this migrationpseudo-tty/repl-dumb-tty.js: There's not currently a way to conditionally run these tests (run only when inspector)Fixes: #61390
Fixes: #36047
Fixes: #38503
Fixes: #39387 (Closes #39392)
Fixes: #37445
Fixes: #38145
Fixes: #33369
Fixes: #48131
Fixes: #8309
Fixes: #39689
Closes #63879
Notable Change
The REPL has been re-implemented on top of the V8 Inspector (
node:inspector)rather than
node:vm. This moves the REPL from a synchronous evaluator to anasynchronous one and resolves a long-standing design quirks that stemmed from the
vm-based design:supports top-level
await, the--no-experimental-repl-awaitflag is nolonger accepted, and top-level
awaitno longer corrupts the lexical scoping ofconst/letdeclarations.import { readFile } from 'node:fs/promises'are now transformed into real dynamic imports and executed._and_errorare available even in REPLs with a modified context.add listeners (e.g.
uncaughtException) without disrupting evaluation.queueMicrotask/timer callbacks, andvmasync timeouts, no longer take down the process.Breaking changes
In addition to the removal of
--no-experimental-repl-awaitnoted above:inspector, their output (including added detail) can differ from previous
releases. Code or tests that match on exact REPL error/stack-trace text may
need to be updated.
SyntaxErroris no longer emitted in the main REPL, however, this information is still available programmatically in the stack trace.