feat(install): add --lockfile-only flag
#31376
Conversation
WalkthroughAdds a global --lockfile-only flag and threads it through add, install, remove/uninstall, outdated/update, and approve-scripts CLI paths. Refactors install flags into InstallTopLevelFlags and InstallEntrypointsFlags and updates installer call sites (install_from_entrypoints, install_top_level). Introduces CacheTopLevelDepsOptions to control cache/install behavior. Reworks the npm installer API to accept NpmInstallerOptions and adds install_resolution_if_pending / install_if_pending entry points. Adds tests for lockfile-only workflows and updates workspace deno_npm from =0.42.0 to =0.42.1. Sequence Diagram(s)sequenceDiagram
actor User
participant CLI
participant Args as Args/Flags
participant Installer
participant NpmRes as NpmResolutionInstaller
participant Cache
User->>CLI: add --lockfile-only npm:pkg
CLI->>Args: parse --lockfile-only
Args-->>CLI: AddFlags{lockfile_only:true}
CLI->>Installer: install_from_entrypoints(flags, InstallEntrypointsFlags{entrypoints, lockfile_only:true})
Installer->>NpmRes: install_if_pending()
NpmRes->>Cache: resolve & write lockfile
Cache-->>NpmRes: done
NpmRes-->>Installer: resolution complete (no node_modules)
Installer-->>CLI: report success
CLI-->>User: deno.lock updated, node_modules untouched
sequenceDiagram
participant UpdateCmd
participant PM
participant CacheDeps as cache_top_level_deps
participant NPMInstall
UpdateCmd->>PM: OutdatedKind::Update{lockfile_only:true}
PM->>CacheDeps: cache_top_level_deps(CacheTopLevelDepsOptions{lockfile_only:true})
alt lockfile_only
CacheDeps->>NPMInstall: install_resolution_if_pending()
else
CacheDeps->>NPMInstall: cache_packages (full install)
end
NPMInstall-->>CacheDeps: done
CacheDeps-->>UpdateCmd: return
sequenceDiagram
participant Caller
participant Options as NpmInstallerOptions
participant NpmInstaller
Caller->>Options: construct NpmInstallerOptions{maybe_lockfile,...}
Caller->>NpmInstaller: NpmInstaller::new(install_reporter, Options)
NpmInstaller-->>Caller: initialized (supports install_resolution_if_pending)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/args/flags.rs (1)
3352-3391: Wire--lockfile-onlyconsistently forupdate/uninstalland fix one outdated test expectationThere are a couple of inconsistencies between CLI wiring, parsers, and tests:
deno update --lockfile-only:
update_parsereadsmatches.get_flag("lockfile-only")viaoutdated_parse(OutdatedKind::Update haslockfile_only), and tests expect--lockfile-onlyto be supported forupdate(see theupdate_subcommandcases expectinglockfile_only: true).- However,
update_subcommand()only callsupdate_and_outdated_args()andlock_args(); it never registerslockfile-only, so Clap will treat--lockfile-onlyas an unknown argument and the test will fail.
deno uninstall --lockfile-only:
uninstall_parse()setsRemoveFlags { lockfile_only: matches.get_flag("lockfile-only") }for the local uninstall path, and the newuninstalltest with--lockfile-onlyexpects this to work.- But
uninstall_subcommand()never addslockfile_only_arg(), so--lockfile-onlyis again an unknown argument and can never reachuninstall_parse().Outdated test expecting
latest: truewithout--latest:
- In the
outdated_subcommandtests, the case for["--update", "--lockfile-only"]currently expectsOutdatedKind::Update { latest: true, interactive: false, lockfile_only: true }, butoutdated_parse()only setslatestbased on--latest. With no--latestflag,latestwill befalse, so this assertion is wrong.Concrete fixes:
- In
update_subcommand()(around Line 3352), register the flag:.defer(|cmd| { cmd .args(update_and_outdated_args()) + .arg(lockfile_only_arg()) .arg( Arg::new("interactive")
- In
uninstall_subcommand()(around Line 3446), register the flag for local uninstalls:.defer(|cmd| { cmd .arg(Arg::new("name-or-package").required_unless_present("help")) ... .arg( Arg::new("additional-packages") .help("List of additional packages to remove") .conflicts_with("global") .num_args(1..) .action(ArgAction::Append) ) .args(lock_args()) + .arg(lockfile_only_arg()) })
- In the
outdated_subcommandtests (around Line 13342), adjust the["--update", "--lockfile-only"]expectation to keeplatest: falseand only togglelockfile_only:- OutdatedFlags { - filters: vec![], - kind: OutdatedKind::Update { - latest: true, - interactive: false, - lockfile_only: true, - }, - recursive: false, - }, + OutdatedFlags { + filters: vec![], + kind: OutdatedKind::Update { + latest: false, + interactive: false, + lockfile_only: true, + }, + recursive: false, + },With these changes, the CLI, parse logic, and tests should all agree on
--lockfile-onlybehavior forupdateanduninstall, and the outdated test will reflect howlatestis actually derived.Also applies to: 3446-3487, 5400-5405, 5408-5436, 6117-6137, 10072-10107, 12872-12884, 13341-13352, 13488-13500
🧹 Nitpick comments (1)
cli/args/flags.rs (1)
3264-3269: Make--lockfile-onlyhelp text command-agnostic
lockfile_only_arg()’s help string is “Install only updating the lockfile”, but the same arg is reused ondeno add,deno remove,deno outdated --update, and (via shared parsing)deno update. For those commands it’s not “installing,” it’s “updating the lockfile without changing local installs.”Consider rewording to something neutral like “Only update the lockfile; do not change locally installed dependencies,” or give each subcommand its own brief help text if you want to be precise per verb.
Also applies to: 2055-2085, 2157-2182, 3166-3262, 3393-3443
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
Cargo.lockis excluded by!**/*.locktests/specs/add/lockfile_only/install.outis excluded by!**/*.outtests/specs/add/lockfile_only/remove.outis excluded by!**/*.outtests/specs/install/lockfile_only/install.outis excluded by!**/*.outtests/specs/install/lockfile_only_entrypoint/install.outis excluded by!**/*.outtests/specs/outdated/deno_json/update_latest_lockfile_only/update.outis excluded by!**/*.out
📒 Files selected for processing (18)
Cargo.toml(1 hunks)cli/args/flags.rs(32 hunks)cli/args/mod.rs(1 hunks)cli/lsp/resolver.rs(2 hunks)cli/main.rs(1 hunks)cli/tools/installer/mod.rs(10 hunks)cli/tools/pm/cache_deps.rs(2 hunks)cli/tools/pm/mod.rs(5 hunks)cli/tools/pm/outdated/mod.rs(4 hunks)libs/npm_installer/factory.rs(3 hunks)libs/npm_installer/lib.rs(6 hunks)libs/npm_installer/resolution.rs(1 hunks)tests/specs/add/lockfile_only/__test__.jsonc(1 hunks)tests/specs/install/lockfile_only/__test__.jsonc(1 hunks)tests/specs/install/lockfile_only_entrypoint/__test__.jsonc(1 hunks)tests/specs/install/lockfile_only_entrypoint/deno.json(1 hunks)tests/specs/install/lockfile_only_entrypoint/main.ts(1 hunks)tests/specs/outdated/deno_json/__test__.jsonc(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
cli/main.rs (1)
cli/tools/installer/mod.rs (1)
install_from_entrypoints(429-464)
cli/tools/pm/cache_deps.rs (3)
cli/lsp/resolver.rs (1)
npm_installer(1137-1139)libs/npm_installer/factory.rs (1)
npm_installer(345-385)cli/factory.rs (1)
npm_installer(641-643)
libs/npm_installer/resolution.rs (3)
libs/npm_installer/lib.rs (1)
add_package_reqs(265-275)libs/resolver/npm/managed/resolution.rs (4)
package_reqs(128-136)snapshot(107-112)snapshot(139-144)snapshot(171-173)cli/lsp/resolver.rs (2)
snapshot(194-266)snapshot(514-523)
cli/tools/pm/mod.rs (1)
cli/tools/pm/cache_deps.rs (1)
cache_top_level_deps(30-233)
cli/lsp/resolver.rs (3)
libs/npm_installer/factory.rs (1)
maybe_lockfile(233-241)cli/factory.rs (2)
maybe_lockfile(369-373)new(123-134)libs/resolver/factory.rs (1)
maybe_lockfile(479-518)
libs/npm_installer/lib.rs (3)
cli/factory.rs (4)
install_reporter(623-639)new(123-134)npm_cache(547-549)sys(517-519)libs/npm_installer/resolution.rs (1)
new(90-109)libs/npm_installer/factory.rs (3)
new(113-139)npm_cache(243-253)tarball_cache(402-418)
cli/tools/installer/mod.rs (3)
cli/factory.rs (2)
from_flags(340-347)emitter(729-731)cli/util/display.rs (3)
human_elapsed(51-53)human_elapsed_with_ms_limit(55-67)format(22-23)cli/tools/pm/cache_deps.rs (1)
cache_top_level_deps(30-233)
libs/npm_installer/factory.rs (2)
cli/factory.rs (1)
workspace_factory(650-673)libs/resolver/factory.rs (2)
workspace_factory(1152-1154)workspace_npm_link_packages(634-650)
🔇 Additional comments (19)
tests/specs/install/lockfile_only_entrypoint/deno.json (1)
1-3: LGTM!The configuration appropriately sets up manual node_modules handling for the lockfile-only entrypoint test scenario.
tests/specs/install/lockfile_only_entrypoint/main.ts (1)
1-1: LGTM!Simple test fixture that imports an npm package to exercise lockfile-only behavior with entrypoints.
tests/specs/install/lockfile_only_entrypoint/__test__.jsonc (1)
1-17: LGTM!Test specification comprehensively verifies that
--lockfile-onlywith entrypoints updates the lockfile without creating node_modules.tests/specs/install/lockfile_only/__test__.jsonc (1)
1-30: LGTM!Test specification thoroughly validates both lockfile-only mode (no node_modules) and normal install mode (creates node_modules).
tests/specs/add/lockfile_only/__test__.jsonc (1)
1-30: LGTM!Test specification comprehensively validates add and remove commands with
--lockfile-only, ensuring lockfile updates without node_modules creation.tests/specs/outdated/deno_json/__test__.jsonc (1)
115-134: LGTM!New test case validates
outdated --update --latest --lockfile-onlycommand. The output paths referencingupdate_latest(rather thanupdate_latest_lockfile_only) for deno.json and deno.lock comparisons suggest these files should be identical between modes.cli/args/mod.rs (1)
1374-1379: LGTM!Correctly extends the Manual caching strategy to include entrypoint installs with
lockfile_only: true, ensuring npm packages aren't installed locally when the flag is set.Cargo.toml (1)
74-74: No issues found.The deno_npm version "0.42.1" is valid and available on crates.io.
cli/tools/pm/cache_deps.rs (1)
26-28: Lockfile-only path correctly bypasses npm package cachingThe new
CacheTopLevelDepsOptions { lockfile_only }plus the conditional at the end cleanly preserves the old behavior whenlockfile_onlyis false and, when true, limits work toinstall_resolution_if_pending()instead ofcache_packages(PackageCaching::All), which is exactly what we want for a lockfile-only flow.Also applies to: 34-35, 222-228
cli/main.rs (1)
155-163: Cache subcommand wiring to InstallEntrypointsFlags looks correctPassing
InstallEntrypointsFlags { entrypoints: cache_flags.files, lockfile_only: false }is a straight adaptation to the new API and keepsdeno cachesemantics unchanged.cli/tools/pm/mod.rs (1)
44-46: lockfile_only is consistently threaded through add/remove into npm install flowRe-exporting
CacheTopLevelDepsOptions, extendingnpm_install_after_modificationto acceptcache_options, and populating it fromadd_flags.lockfile_only/remove_flags.lockfile_onlyensures the new CLI flag affects the cache/install behavior only where intended, while keeping the non-flagged path identical.Also applies to: 586-594, 916-924, 929-947
libs/npm_installer/factory.rs (1)
24-25: NpmInstallerOptions construction correctly replaces positional argumentsThe refactor to pass
NpmInstallerOptions { maybe_lockfile, maybe_node_modules_path, lifecycle_scripts, system_info, workspace_link_packages }intoNpmInstaller::newkeeps the same data flow as before, just structured, and should be behaviorally identical.Also applies to: 357-380
libs/npm_installer/resolution.rs (1)
119-123: install_if_pending + inner helper preserve behavior; confirm pending-flag contractThe split into
add_package_reqs_inner()looks good: it centralizes the queue lock, runsadd_package_reqs_to_snapshot(), and on success clears the pending flag and updates the snapshot.add_package_reqs()now simply delegates and convertsNpmResolutionErrorintoJsErrorBox, so its observable behavior is unchanged.install_if_pending()reuses the same path with an empty package list, which will only trigger real work when the resolution is in a state whereadd_package_reqs_to_snapshot()decides to run (for example, when marked pending).One thing worth double-checking is that every path which should cause
--lockfile-onlyoperations to recompute and write the lockfile is setting the resolution into that “pending” state beforeinstall_if_pending()is called; otherwise the empty-req call here will be a no-op. If your new tests coverdeno add/remove/... --lockfile-onlyend-to-end, that should validate the contract.Also applies to: 125-135, 137-155
cli/lsp/resolver.rs (1)
931-987: LSP NpmInstaller options wiring looks consistent and safeUsing
NpmInstallerOptionshere looks correct: LSP explicitly passesmaybe_lockfile: None, so it won’t mutate the project lockfile, while still wiringmaybe_node_modules_path,workspace_link_packages, and default lifecycle/system info through the structured options. This keeps previous behavior while matching the new constructor.cli/tools/pm/outdated/mod.rs (1)
18-18: Lockfile-only behavior fordeno outdated --updateis wired correctlyThe new
lockfile_onlyflag is threaded cleanly fromOutdatedKind::Updateintoupdateand then intonpm_install_after_modificationviaCacheTopLevelDepsOptions. That keeps the existing update logic while allowing lockfile-only resolution without touching installed npm packages.Also applies to: 252-267, 360-367, 436-440
cli/tools/installer/mod.rs (2)
207-215: Resolution reporting and “Resolved” summary align with lockfile-only behaviorUsing
package_req.to_string()inon_resolvedto populateresolved_npmand adding the “Resolved … in ” branch inprint_install_reportis a good fit for lockfile-only flows: when there are no npm/JSR downloads or initializations, users still see a clear summary of how many packages were resolved and how long it took, while install/download cases continue to use the existing “Installed/Downloaded” path.Also applies to: 584-595, 611-677
40-45: Structured flags and lockfile-only wiring for install flows look solidSwitching to
InstallEntrypointsFlags/InstallTopLevelFlagssimplifies the call sites, and havinginstall_top_levelown theCliFactoryconstruction and timing makes the API easier to use. Propagatingtop_level_flags.lockfile_onlyintoCacheTopLevelDepsOptionsensuresdeno install’s top-level mode honors the new lockfile-only semantics without changing existing behavior when the flag is false.Also applies to: 429-432, 437-439, 471-479, 740-780
libs/npm_installer/lib.rs (2)
148-154: NpmInstallerOptions refactor cleanly encapsulates installer configurationMoving
maybe_lockfile,maybe_node_modules_path, lifecycle script config, system info, and workspace link packages intoNpmInstallerOptionsand threading that throughNpmInstaller::newis a good cleanup: it shortens the constructor, keeps lockfile handling explicit on the installer viamaybe_lockfile, and cleanly separates local vs global installer setup based onmaybe_node_modules_pathwithout changing the observable behavior.Also applies to: 173-236
425-476:install_resolution_if_pendingis an appropriate hook for lockfile-only installsThe new
install_resolution_if_pendinghelper cleanly separates “finalize npm resolution state if there’s a pending snapshot” from package caching. Calling throughnpm_resolution_installer.install_if_pending()after ensuring initialization gives lockfile-only callers a way to update the resolution (and lockfile via the installer) without invokingcache_packages, which matches the intended semantics.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cli/tools/installer/mod.rs (1)
591-593: Consider inlining or adding a comment.The local
human_elapsedhelper with a 3_000ms limit (vs. the global 1_000ms) is only used once. Consider either inlining the call todisplay::human_elapsed_with_ms_limit(elapsed.as_millis(), 3_000)directly at line 611, or adding a comment explaining why the different limit is needed for install reporting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
tests/specs/add/lockfile_only/install.outis excluded by!**/*.outtests/specs/install/lockfile_only/install.outis excluded by!**/*.outtests/specs/install/lockfile_only_entrypoint/install.outis excluded by!**/*.out
📒 Files selected for processing (1)
cli/tools/installer/mod.rs(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cli/tools/installer/mod.rs (3)
cli/factory.rs (2)
from_flags(340-347)emitter(729-731)cli/util/display.rs (2)
human_elapsed(51-53)human_elapsed_with_ms_limit(55-67)cli/tools/pm/cache_deps.rs (1)
cache_top_level_deps(30-233)
🔇 Additional comments (5)
cli/tools/installer/mod.rs (5)
40-40: LGTM!New imports for the structured flag types are correctly added and used throughout the file.
Also applies to: 44-44
210-213: LGTM!The parameter rename and usage update correctly tracks the package requirement instead of leaving it unused.
429-464: LGTM!The refactor to accept structured
InstallEntrypointsFlagsis clean and the entrypoints are correctly accessed from the struct.
477-479: LGTM!The new match arm for TopLevel flags follows the existing pattern and correctly forwards parameters.
723-763: LGTM!The refactor correctly:
- Moves factory construction into the function for better encapsulation
- Threads the new
lockfile_onlyflag through tocache_top_level_deps- Improves timing variable naming and usage consistency
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/args/flags.rs (1)
2069-2099:lockfile-onlyhelp text is install-specific but reused across multiple commands
lockfile_only_arg()’s help"Install only updating the lockfile"is attached toadd,remove,install,uninstall,outdated,update, andapprove-scripts. For non-installcommands this is misleading (e.g.,deno outdated --lockfile-onlyordeno approve-scripts --lockfile-onlydon’t “install” anything).I’d suggest either:
- making the text generic (for example: “Only update the lockfile; do not modify installed dependencies”), or
- providing per-command variants if semantics differ.
This is purely UX/doc, but it will confuse users reading
--help.Also applies to: 2101-2116, 2208-2233, 3217-3313, 3315-3321, 3375-3403, 3431-3443, 3475-3495, 3497-3540
🧹 Nitpick comments (2)
cli/args/flags.rs (2)
5444-5465: Local add/remove/uninstall lockfile-only handling is coherent; consider tightening global uninstall UX
add_parse_innerandremove_parsecorrectly read--lockfile-onlyintoAddFlags/RemoveFlags.uninstall_parseonly useslockfile_onlyfor the local branch, which matches the intent that lockfile updates are a local dependency concern.- Tests for
uninstallwith--frozen --lockfile-onlyand for multiple package removal confirm the parser behavior.One minor UX edge case:
uninstall_subcommandcurrently allowsdeno uninstall --global --lockfile-onlywithout a clap-level conflict, but the flag is then ignored in the global branch. For consistency withinstall, you might want to add.conflicts_with("global")to the uninstalllockfile_only_arg()usage, or explicitly document that it’s ignored for global uninstalls.Also applies to: 5467-5473, 6185-6204, 2208-2233, 3497-3540, 10205-10239
3375-3403: Outdated/update: lockfile-only threading is good; optional tightening around non-update usageThe shared
update_and_outdated_args()plusoutdated_parse/update_subcommandchanges cleanly thread--lockfile-onlyintoOutdatedKind::Update { .. }, and the tests coveroutdated --update --lockfile-onlyandupdate --lockfile-onlywell.Today
--lockfile-onlyis accepted even when no update occurs (e.g.,deno outdated --lockfile-only), where it’s effectively a no-op. If you want stricter UX, you could consider.requires("update")on this arg inoutdated_subcommand(and rely on the dedicatedupdatesubcommand for the other path), but functionally the current behavior is safe.Also applies to: 3445-3495, 5475-5505, 13606-13614, 13527-13614
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
Cargo.toml(1 hunks)cli/args/flags.rs(38 hunks)cli/args/mod.rs(1 hunks)cli/main.rs(1 hunks)cli/tools/pm/approve_scripts.rs(2 hunks)cli/tools/pm/mod.rs(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- Cargo.toml
- cli/tools/pm/mod.rs
- cli/args/mod.rs
- cli/main.rs
🧰 Additional context used
📓 Path-based instructions (3)
cli/tools/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
CLI tools should be implemented in
cli/tools/<tool>orcli/tools/<tool>/mod.rs
Files:
cli/tools/pm/approve_scripts.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: For debugging Rust code, set breakpoints in IDE debuggers (VS Code with rust-analyzer, IntelliJ IDEA) or uselldbdirectly
Useeprintln!()ordbg!()macros for debug prints in Rust code
Files:
cli/tools/pm/approve_scripts.rscli/args/flags.rs
cli/args/flags.rs
📄 CodeRabbit inference engine (CLAUDE.md)
CLI flag parsing should be defined in
cli/args/flags.rs
Files:
cli/args/flags.rs
🧠 Learnings (1)
📚 Learning: 2025-11-24T16:19:37.808Z
Learnt from: CR
Repo: denoland/deno PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:19:37.808Z
Learning: Applies to cli/args/flags.rs : CLI flag parsing should be defined in `cli/args/flags.rs`
Applied to files:
cli/tools/pm/approve_scripts.rscli/args/flags.rs
🧬 Code graph analysis (1)
cli/tools/pm/approve_scripts.rs (1)
cli/tools/pm/mod.rs (1)
npm_install_after_modification(945-981)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: test release windows-x86_64
- GitHub Check: test release linux-aarch64
- GitHub Check: test debug macos-aarch64
- GitHub Check: test debug windows-x86_64
- GitHub Check: test debug linux-x86_64
- GitHub Check: test debug linux-aarch64
- GitHub Check: lint debug windows-x86_64
- GitHub Check: test debug macos-x86_64
- GitHub Check: lint debug linux-x86_64
- GitHub Check: build libs
- GitHub Check: lint debug macos-x86_64
🔇 Additional comments (7)
cli/tools/pm/approve_scripts.rs (2)
23-23: LGTM!The import is correctly added to support the updated function call below.
136-143: LGTM!The call correctly passes the
lockfile_onlyflag through to the npm install modification flow, aligning with the PR's objective to support lockfile-only operations across commands.cli/args/flags.rs (5)
113-118: Lockfile-only flag plumbing looks consistent across structs and enumsThe added
lockfile_onlyfields onAddFlags,RemoveFlags,ApproveScriptsFlags,InstallTopLevelFlags,InstallEntrypointsFlags, andOutdatedKind::Updateare wired consistently and give a clear data path for the new behavior. No structural issues spotted in these definitions.Also applies to: 133-136, 611-617, 631-634, 320-329, 3315-3321
1269-1288: Config discovery forinstall --entrypointviaconfig_path_argsis reasonableReusing the cache logic to resolve an entrypoint’s directory (and falling back to
current_dirwhen resolution fails) is a sensible way to infer config roots fordeno install --entrypoint .... The “first entrypoint only” behavior for bothcacheandinstallis consistent with the updated tests and looks acceptable.
2119-2135: Approve-scriptslockfile-onlywiring and tests look correctParsing
--lockfile-onlyintoApproveScriptsFlags.lockfile_onlyand covering combinations inapprove_scripts_subcommand()tests (no packages, multiple packages, comma-separated values, flag position variance) looks solid. The behavior is coherent with the other dependency-management commands.Also applies to: 631-634, 13616-13696
5998-6072: Install local/global split and new local variants look soundThe revised
install_parsecleanly separates global from local cases and uses the newInstallTopLevelFlags/InstallEntrypointsFlagsto carrylockfile_only. Theconflicts_with("global")onlockfile-onlyfor the install subcommand avoids meaningless combinations, and theconfig_path_argstests fordeno install -e sub/test.jsconfirm the entrypoint path handling. No issues spotted here.Also applies to: 320-329, 12044-12048
7310-7313: New and updated tests give good coverage of the lockfile-only behaviorThe added assertions in
uninstall,add_or_install_subcommand,outdated_subcommand,update_subcommand, andapprove_scripts_subcommandtests exercise:
- presence/absence of
lockfile_onlyon all relevant flag structs,- interaction with
--frozen,- multiple packages and comma-separated forms.
This gives solid confidence in the new parsing paths.
Also applies to: 10205-10239, 10889-10936, 13527-13614, 13616-13696
| if options.lockfile_only { | ||
| // do a resolution install if the npm snapshot is in a | ||
| // pending state due to a config file change | ||
| npm_installer.install_resolution_if_pending().await?; | ||
| } else { | ||
| npm_installer.cache_packages(PackageCaching::All).await?; | ||
| } |
There was a problem hiding this comment.
This appears to be the clue of this PR
2 participants
Adds a
--lockfile-onlyflag that only updates the lockfile and doesn't install any npm deps locally. Note that jsr and https dependencies are still cached because they require looking at the code to resolve dependencies.Applies to
deno install,deno add,deno remove,deno outdated --update, anddeno updateCloses #31371