UpgradeDependencyVersion: warn on install failure instead of throwing#7479
Merged
Jenson3210 merged 1 commit intomainfrom Apr 27, 2026
Merged
UpgradeDependencyVersion: warn on install failure instead of throwing#7479Jenson3210 merged 1 commit intomainfrom
Jenson3210 merged 1 commit intomainfrom
Conversation
When the package-manager install run by `UpgradeDependencyVersion` fails (typically an `ERESOLVE` peer-dep conflict), the recipe currently throws, which aborts the entire recipe chain. That prevents later recipes — which might be the very ones that resolve the conflict, e.g. by bumping a peer-pinning library — from running at all. The sibling `AddDependency` recipe already handles install failure by returning `markupWarn(doc, ...)` and letting the chain continue. Align the two so partial failures degrade gracefully and downstream recipes have a chance to converge the dependency graph. Real-world trigger: an Angular 17 project with `ng-bootstrap@16` (peer `@angular/core@^17`) bumped via a chain of `UpgradeDependencyVersion` calls. Today the first failed install kills the run; with this change, the chain continues, the marker records the failure, and a later `UpgradeDependencyVersion` for the conflicting peer can succeed. Tests: the two existing tests that asserted on the throw are reworked to assert on the `MarkupWarn` marker instead, mirroring the pattern in `add-dependency.test.ts::adds warning marker when package does not exist`.
steve-aom-elliott
approved these changes
Apr 27, 2026
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.
Summary
When the package-manager install run by
UpgradeDependencyVersionfails (typically anERESOLVEpeer-dep conflict), the recipe currentlythrows, aborting the entire recipe chain. That prevents later recipes — which might be the very ones that resolve the conflict, e.g. by bumping a peer-pinning library — from running at all.The sibling
AddDependencyrecipe already handles install failure by returningmarkupWarn(doc, ...)and letting the chain continue. This PR aligns the two so partial failures degrade gracefully.Motivation
Real-world trigger: an Angular 17 project with
@ng-bootstrap/ng-bootstrap@^16.0.0(strict peer@angular/core@^17.0.0) being upgraded via a chain ofUpgradeDependencyVersioncalls (one for@angular/*, one for@ng-bootstrap/ng-bootstrap). Today the first failing install kills the run, and the dep graph stays in its initial inconsistent state forever. With this change, the chain continues, the marker records the failure, and a subsequent recipe can converge the graph (or a second pass of the same chain reaches fixed-point).Diff
Tests
The two existing tests that asserted on the throw —
throws when install fails for non-existent versionandthrows when install fails due to engine version mismatch— are reworked to assert on theMarkupWarnmarker, mirroring the pattern inadd-dependency.test.ts::adds warning marker when package does not exist. Renamed toadds warning marker when install fails …to match the new behaviour.All 25 tests in
upgrade-dependency-version.test.tspass locally.Test plan
npm run typecheckcleannpx vitest run test/javascript/recipes/upgrade-dependency-version.test.ts— 25/25 pass