[WIP] repl: use inspector over vm#64034
Draft
avivkeller wants to merge 1 commit into
Draft
Conversation
Collaborator
|
Review requested:
|
a6b73a5 to
b00e3d5
Compare
86da550 to
64c8877
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 a synchronous evaluator to a async one1:--experimental-repl-awaitis no longer required, since top-levelawaitis no longer experimental and is natively supported by the inspector.Static imports such as
import ... from '...'are now transformed into real dynamic imports and executed, instead of throwingCannot use import statement outside a modulewith a hint to rewrite the import manually. This is not strictly required by removingnode:vm, but it is made easier by the change. I can split this into a separate PR if preferred.Since we are no longer relying on
node:vm, several issues with VM have been resolved (see the list at the bottom of this PR), and users can now attach process listeners without breaking the REPL.The REPL now gives more details in Errors, for instance, showing the specific line in a
.load-ed file where an error was thrownOpening as a draft for reviews while I work on changing tests to support the async behavior of the new REPL
Note on the removed tests:
test/parallel/test-repl-domain.js: The REPL now functions completely independently from thedomainmodule, and, as such, there's no reason to test it's functionality in the REPLtest/parallel/test-repl-uncaught-exception-async.js/test/parallel/test-repl-uncaught-exception.js/test-repl-tab-complete-nested-repls.js: We no longer throw on these cases (yay!)test/parallel/test-repl-no-terminal-restore-process-listeners.js: We no longer block the setting of listeners on the REPL.test/parallel/test-repl-preprocess-top-level-await.js/test/parallel/test-repl-top-level-await.js: We no longer have custom logic for Top Level Await, so testing it would be testing code already tested internally by the inspector.Fixes: #36047
Fixes: #38503
Fixes: #39387
Fixes: #37445
Fixes: #38145
Fixes: #33369
Fixes: #8309
Footnotes
This is a breaking change. ↩