-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Angular: Add support for v21 #33098
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Angular: Add support for v21 #33098
Conversation
|
View your CI Pipeline Execution ↗ for commit 0d95f7b
☁️ Nx Cloud last updated this comment at |
… schema conditionally to make Angular prerelease sandboxes working again
ba8dd57 to
7b6f0fb
Compare
📝 WalkthroughWalkthroughUpdate adds semver-based devkit version resolution in the Angular Storybook generator, adds extra Angular runtime dependencies in sandbox templates, and widens Angular peerDependency ranges while adding Changes
Sequence Diagram(s)Skipped — changes are dependency and configuration updates without control-flow modifications. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
✨ Finishing touches
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-09-17T07:31:04.432ZApplied to files:
⏰ 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). (2)
🔇 Additional comments (2)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
code/lib/create-storybook/src/generators/ANGULAR/index.ts (2)
8-8: Semver import & robustness ofminVersionusageUsing a default import for
semverplussemver.minVersion(angularVersion)looks fine assuming the repo is compiled withesModuleInterop/allowSyntheticDefaultImportsandpackageManager.getDependencyVersionalways returns a semver-compatible range.To make this more robust and avoid surprises with non-standard specs (e.g. aliases or tags), consider:
- Guarding the call with
semver.validRangebeforeminVersion, and- Optionally falling back to
false(no@standard-schema/spec) if the range is not valid instead of risking a runtime error.For example:
const needsStandardSchema = (() => { if (!angularVersion || !semver.validRange(angularVersion)) { return false; } const min = semver.minVersion(angularVersion); return !!min && min.major >= 21; })();Also worth quickly checking how
semveris imported elsewhere in the repo to keep the style consistent (default vs namespace import).Also applies to: 100-108
62-62: Check assumptions about tying devkit/platform-browser-dynamic versions to@angular/coreDeriving
@angular-devkit/*and@angular/platform-browser-dynamicversions directly from the detected@angular/coreversion is a reasonable heuristic and should help prerelease sandboxes, but it does bake in an assumption that these packages track the same version range as@angular/core.Two follow-ups you may want to consider:
- If the workspace already has
@angular-devkit/build-angular(or the others) installed with a different but compatible range, you might prefer to read that version first and only fall back to@angular/corewhen no devkit dependency is present.- If
angularVersionis a very loose range (e.g.^21.0.0-0), you may want to keep these extra deps unpinned (no@${angularVersion}suffix) to avoid inadvertently forcing upgrades/downgrades when Storybook is added.If your testing against typical Angular 17–21 CLI workspaces shows this mapping behaves well, it’s fine as-is, but documenting this assumption in the code comment would help future maintainers.
Also applies to: 89-98
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/lib/create-storybook/src/generators/ANGULAR/index.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32484
File: code/core/package.json:326-326
Timestamp: 2025-09-17T07:31:04.432Z
Learning: In Storybook's core package, dependencies like `open` are bundled into the final distribution during the build process, so they should remain in devDependencies rather than being moved to dependencies. End users don't need these packages as separate runtime dependencies since they're included in the bundled code.
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/lib/create-storybook/src/generators/ANGULAR/index.ts
⏰ 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). (2)
- GitHub Check: daily
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/lib/create-storybook/src/generators/ANGULAR/index.ts (1)
110-115: Conditional@standard-schema/specfor Angular ≥21 looks good; just confirm the peer rangeThe
needsStandardSchemagate combined with:...(needsStandardSchema ? ['@standard-schema/spec@^1'] : []),is a clean way to ensure Angular 21+ projects get the required peer without impacting older workspaces. Assuming
@angular/forms@21’s peer really is@standard-schema/spec@^1, this should resolve the prerelease sandbox issue.I’d just suggest:
- Double-checking the exact peer range in the Angular 21 forms package, and
- (Optionally) adding a brief comment including that peer range so future updates (e.g. if Angular 22 moves to
^2) are easier to spot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code/lib/cli-storybook/src/sandbox-templates.ts(1 hunks)code/lib/create-storybook/src/generators/ANGULAR/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- code/lib/create-storybook/src/generators/ANGULAR/index.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: mrginglymus
Repo: storybookjs/storybook PR: 32556
File: code/core/package.json:309-313
Timestamp: 2025-09-29T13:20:23.346Z
Learning: The `fast-printf` dependency in Storybook's core package is bundled into the final distribution during the build process, so it should remain in devDependencies rather than being moved to dependencies, following the same pattern as other bundled dependencies like `open`.
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32484
File: code/core/package.json:326-326
Timestamp: 2025-09-17T07:31:04.432Z
Learning: In Storybook's core package, dependencies like `open` are bundled into the final distribution during the build process, so they should remain in devDependencies rather than being moved to dependencies. End users don't need these packages as separate runtime dependencies since they're included in the bundled code.
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
⏰ 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). (2)
- GitHub Check: daily
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/lib/cli-storybook/src/sandbox-templates.ts (1)
589-589: Package name and version specifier verified as correct.The web query confirms that Angular 21's @angular/forms package expects @standard-schema/spec@^1.0.0 as a peer dependency. The code specifies @standard-schema/spec@^1, which is equivalent in npm semver (both allow any 1.x version) and aligns with Angular's requirements. No changes needed.
472cab9 to
fcbd2c7
Compare
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 191 | 191 | 0 |
| Self size | 118 KB | 118 KB | 🎉 -43 B 🎉 |
| Dependency size | 30.22 MB | 30.31 MB | 🚨 +93 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
yannbf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you <3
Closes #
What I did
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.