fix(node_shim): forward --require/--import to translated deno args#34637
Open
bartlomieju wants to merge 2 commits into
Open
fix(node_shim): forward --require/--import to translated deno args#34637bartlomieju wants to merge 2 commits into
bartlomieju wants to merge 2 commits into
Conversation
node_shim parsed Node's --require/-r and --import flags into preload_cjs_modules/preload_esm_modules but translate_to_deno_args never emitted them, so the preload modules were silently dropped from the translated `deno run`/`deno eval`/`deno test` invocation. deno run, eval, and test all accept --require (CommonJS preload) and --import (alias of --preload, ESM preload) via runtime_misc_args, so forward each parsed preload module on every translated path. Add an add_preload_flags helper called from add_common_flags (run/test) and the eval path, and cover run, eval, short -r, and ordering with unit tests.
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.
node_shimparses Node's--require/-rand--importflags intopreload_cjs_modulesandpreload_esm_modules, buttranslate_to_deno_argsnever read those vectors back out. The result was that a translated
nodeinvocation silently dropped its preload modules:
node --require ./setup.js script.jsbecamedeno run ... script.jswith the--requiregone.deno run,deno eval, anddeno testall accept--require(a CommonJSpreload) and
--import(an alias of--preload, an ES module preload) throughruntime_misc_args, so the parsed preload modules can be forwarded directly onevery translated path. This adds an
add_preload_flagshelper that emits a--requirefor each CommonJS preload and an--importfor each ESM preload,wired into
add_common_flags(covering the run and test paths) and into theeval path (which builds its flags inline). The preload flags are emitted before
the script or eval code so they preload ahead of the main module, matching
Node.
The REPL path is intentionally left unchanged; preload-into-REPL is a separate
case from the script and eval translations this targets.
New unit tests cover forwarding for
--require,-r, and--importon the runpath, multiple preloads preserving order, and
--requirecombined with-e(asserting the flag lands on
deno evaland precedes the eval code).This came out of reviewing #34605, which hand-rolled a partial
nodeargumentparser in
cli/task_runner.rspartly becausenode_shimdropped these flags.Fixing it here lets that path delegate to
node_shiminstead of reimplementingthe parsing.