Repository navigation
Angular: Raise the supported floor to 21 - #36166
Conversation
obvious-autobuild
Bot
added
maintenance
41a873d to
083b5b6
Compare
1dbb4fe to
1eed720
Compare
…ad after backoff Attempt 5 of the flaky benchmark-packages BigQuery upload (4 failures / 1 success over 2.5h today, repo-wide per PR #36166). Retrying after a ~25-minute backoff per the error's own guidance.
2a7513a to
d5b7e73
Compare
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
|
@ObviousApp Resolve conflicts |
d5b7e73 to
44ce3b3
Compare
|
Done — the branch is rebuilt on Two Pushed with |
![obvious-autobuild[bot]](https://avatars.githubusercontent.com/in/2905616?s=60&v=4){kind=small}
There was a problem hiding this comment.
Obvious Code Review
Verdict: COMMENT (2 Medium findings — advisory, non-blocking)
Review of the canonical patch at head 44ce3b3 (includes the RxJS ^6.5.3 arm restore). All prior maintainer feedback from valentinpalkovic verified applied — none re-raised.
Medium
code/frameworks/angular/src/client/angular-beta/utils/PropertyExtractor.ts:185— the new 21 floor also makes two siblingVERSION.major >= 21checks constant-true in the builders; the PR body's "last version-conditional code path" claim doesn't holdcode/frameworks/angular/package.json:100— devDependencies still pin Angular^19.1.1, below the floor this PR enforces; the reported green test suite ran against an unsupported Angular
Suggestions
code/frameworks/angular/package.json:105— typescript peer (^4.9.0 || ^5.0.0 || ^6.0.0) drifts fromangular-vite's>= 5.9.x, contrary to the PR body's byte-for-byte claim
Verified clean: upgrade-blocker flow (semver.lt boundaries at 20/21/22, missing package, --force untouched), MIGRATION.md anchor slug, RxJS range vs Angular 21's own peer, byte-for-byte shared-peer match vs angular-vite, VERSION import fully removed, new blocker test coverage, yarn.lock resync.
|
Both Mediums addressed in |
There was a problem hiding this comment.
Obvious Code Review
Verdict: COMMENT (1 Medium finding — advisory)
Incremental pass at head a5a9dffbd3 (devDeps pinned to the supported minimum, builders' zoneless defaults collapsed).
Pass 1 findings — all resolved (verified)
- Constant-true builder checks — both
experimentalZonelessdefaults collapsed totrue,VERSIONimports removed (no residual references), PR body now states one semantics with explicit-falsestill honored - devDependencies on Angular 19 — now
^21.0.0(devkit^0.2100.0,zone.js ^0.16.0) per maintainer request; Angular 19 subtree fully gone from yarn.lock - typescript peer drift — documented as intentional; byte-for-byte claim scoped to Angular-related peers
Medium
code/frameworks/angular/package.json:70—@analogjs/vite-plugin-angularstays on the 1.x line: resolved 1.22.5, whose peer declarations stop at Angular 20 (optional peers, so yarn resolves silently).src/test-setup.tsruns all 187 tests through itssetup-vitestagainst devkit 21.2.22 — a combination the plugin itself declares unsupported. The 2.x line is proven in-repo:angular-viteruns^2.5.2(2.6.1, peers^17..^22devkit,^6..^8vite) with the samesetup-vitestimport.
Verified clean: explicit experimentalZoneless: false preserved; no residual VERSION references; zone.js peers identical across both frameworks; workspace lock entry matches package.json; PR body accurate to the code.
|
The remaining Medium is fixed in |
There was a problem hiding this comment.
Obvious Code Review
Verdict: COMMENT
Incremental pass at head b1d0af3230 (bump of @analogjs/vite-plugin-angular to ^2.5.2).
The Pass 2 Medium is resolved and verified: a single merged lock entry resolving 2.6.1 (the same resolution @storybook/angular-vite runs), zero 1.x residue, devkit 21.2.22 now inside the plugin's declared ^17..^22 peer envelope, and the full suite re-run green through the new setup-vitest harness. All findings from Passes 1–2 are now resolved and verified.
No findings — clean pass.
There was a problem hiding this comment.
Obvious Code Review
Verdict: COMMENT
Incremental pass at head e8857c1373 (yarn.lock dedupe to satisfy the build dedupe gate — the CircleCI fix from the prior head).
Verified: the dedupe is purely mechanical — within-range merges (the Angular descriptor families collapse onto the 21.2.22 resolutions both frameworks' ranges already allowed, which also removes the minor cross-framework version divergence noted earlier) plus pruning of stale orphaned exact-pin entries; the Angular workspace entry, analog 2.6.1, and zone.js 0.16.2 are untouched; no source changes. The queued CircleCI run on this head is the definitive gate check.
No findings — clean pass.
JReinhold
left a comment
JReinhold
left a comment
There was a problem hiding this comment.
approved with minor feedback.
|
@ObviousApp Resolve merge conflicts |
…r 21 floor Bump @angular/* devDependencies ^19.1.1 -> ^21.2.x (devkit ^0.2102.x, build-angular ^21.2.12, zone.js ^0.16.2), matching @storybook/angular-vite, so the package is developed and tested against a supported Angular. The 187-test suite now exercises the same major the peers advertise.
DevDependency ranges now express the floor the peers enforce: @angular/* ^21.0.0, devkit architect ^0.2100.0, zone.js ^0.16.0.
The experimentalZoneless version checks in build-storybook and start-storybook were constant-true under the 21 floor; both now default to true, matching the PropertyExtractor collapse. Explicit experimentalZoneless: false in project options is still honored.
The 1.x line's peer declarations stop at Angular 20 while both peers are optional, so yarn silently pairs it with the Angular 21 toolchain this package now develops against. ^2.5.2 (resolves 2.6.1) is the version @storybook/angular-vite already runs and declares devkit peers ^17..^22.
The lockfile regeneration left 27 duplicate resolutions (Angular devkit, compiler-cli, Babel core among them) that yarn install tolerates but `yarn dedupe --check` rejects in the Build (linux) job.
The 11.0 breaking-change entries in MIGRATION.md double as the
changelog item titles but mixed title formats ("Dropped support for
...", "Require vN and up"). Consolidate them to the dominant
"Scope: <statement>" convention and update the table-of-contents
anchors to match. Body text is unchanged.
Replayed on top of the reverted next: the Next.js 15 section was
removed by the revert of #36167, so its title hunks are dropped.
The MIGRATION.md section title is now "Angular: requires Angular 21 or higher", so the anchor changed. Update the autoblock blocker link and its test assertion so the link resolves instead of dangling.
Fold the separate `if (isStandalone === undefined)` fallback into the declaration as `?? true` per review feedback. The `??` operand is parenthesized because nullish coalescing cannot be mixed with `&&` without parentheses; semantics are unchanged (false is not nullish). Also link the Angular update guide (https://angular.dev/update-guide) from the MIGRATION.md section, mirroring the Next.js section.
74262eb to
f7725f5
Compare
|
Resolved — rebased |
|
Thanks! Both minor items are addressed at f7725f5: the standalone fallback is folded into the declaration ( |
3 participants
Why
Angular 21 is the SB11 minimum, and
@storybook/angularstill advertises>=18.0.0peers — two majors behind@storybook/angular-vite. A webpack-framework project on Angular 18–20 installs cleanly today and breaks only at runtime, because the upgrade-time blocker also stops at 18. The floor has to agree everywhere: package metadata, the upgrade blocker, and the last version-conditional code path.What
code/frameworks/angular/package.json): all Angular-related peers rise to>=21.0.0 < 23.0.0—@angular-devkit/architectgets the devkit form>=0.2100.0 < 0.2300.0.rxjskeeps Angular 21's own peer range^6.5.3 || ^7.4.0(every@angular/core@21.xrelease declares exactly that on the npm registry), so@storybook/angularis never stricter than the framework it integrates. One intentional asymmetry:@storybook/angulartracks Angular's own RxJS range, while@storybook/angular-viteremains RxJS-7-only (^7.4.0). Every Angular-related shared peer still uses the exact string@storybook/angular-vitepublishes, so the two packages can't drift apart again. (Thetypescriptpeer intentionally differs — the webpack framework's^4.9.0 || ^5.0.0 || ^6.0.0predates this change and is untouched here.)code/lib/cli-storybook/src/autoblock/block-dependencies-versions.ts):@angular/coreminimum goes18.0.0→21.0.0. The message stays in the existing style ("Support for Angular < 21 has been removed.") and now links the newMIGRATION.md#angular-requires-angular-21-or-higheranchor instead of the generic angular.dev guide.--forcebypass is untouched — it's the pre-existing!options.forcegate incollectProjects, not blocker logic.>= 19runtime check for the standalone default collapses to the constanttrue(the floor guarantees it), and the now-unusedVERSIONimport goes with it. The two matchingVERSION.major >= 21defaults forexperimentalZonelessin the webpack builders (build-storybook/start-storybook) collapse totruethe same way, so the package carries one semantics: under the 21 floor, zoneless is always supported as the default, and an explicitexperimentalZoneless: falsein a project's options still wins.angular-to-angular-viteis forward-looking (it already requires Angular ≥ 21 and skips below it),angular-vite-remove-compodoconly runs for the@storybook/angular-viteframework, andangular-workspace.tsis a shared helper module, not a registered fix. No fix here exists solely for a pre-21 behavior.How to Review
code/frameworks/angular/package.jsonandcode/lib/cli-storybook/src/autoblock/block-dependencies-versions.tsare the behavioral core; the rest is docs and the dead branch cleanup.block-dependencies-versions.test.tscovers 20 → blocked (with title, message, and the exact MIGRATION.md anchor), 21/22 → pass, and missing-package → pass.< 23.0.0format to matchangular-vitebyte-for-byte.PropertyExtractor.tscarries 8 oxlint warnings that exist on the base commit too; this change adds none.Test Evidence
@storybook/angularsuite: 187 tests, all green.@storybook/angularandangular-vitepasses programmatically (Angular-related peers).@angular/*^21.0.0, devkit^0.2100.0,zone.js^0.16.0), so the suite runs against a supported Angular;yarn.lockresynced andyarn install --immutablepasses.Manual testing
This PR changes package metadata, the upgrade blocker, docs, and one dead code path — there is no rendered UI to exercise by hand. To verify manually:
@angular/core@20, runstorybook upgrade— it stops with "Angular 21 support removed", linking the migration guide; rerunning with--forceproceeds. On@angular/core@21or22, no blocker appears.@angular/core@20and then@storybook/angular@canary— the package manager reports unresolvable peers; with@angular/core@21it installs cleanly.MIGRATION.mdshows the new "Angular: Require v21 and up" entry under "From version 10.x to 11.0.0" (alongside the Vitest 4 and Vite 5/6 entries that landed onnextindependently), with a matching TOC link.🔗 Obvious Project · 🧵 Obvious Thread