feat: update to angular 21 #32
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughUpdates Angular and tooling to 21.x, refactors ESLint export to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✏️ Tip: You can disable this entire section by setting 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: 1
🧹 Nitpick comments (1)
eslint.config.js (1)
23-30: Consider extracting the duplicated rule configuration.The
@typescript-eslint/consistent-type-importsrule is defined identically in both the Storybook files block (lines 23-30) and the TypeScript files block (lines 49-56). While this duplication is intentional due to different file patterns, you could improve maintainability by extracting it to a shared constant.Example refactor:
const typeImportsRule = { '@typescript-eslint/consistent-type-imports': [ 'error', { disallowTypeAnnotations: true, fixStyle: 'inline-type-imports', prefer: 'type-imports', }, ], }; export default defineConfig( { files: ['*.stories.@(ts|tsx|js|jsx|mjs|cjs)', '*.story.@(ts|tsx|js|jsx|mjs|cjs)'], // ... rules: { ...typeImportsRule, // other rules... }, }, { files: ['**/*.ts'], // ... rules: { ...typeImportsRule, // other rules... }, }, // ... );Also applies to: 49-56
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
eslint.config.js(3 hunks)package.json(3 hunks)src/login/KcPageStory.ts(1 hunks)src/main.ts(1 hunks)tsconfig.json(1 hunks)
🔇 Additional comments (5)
src/login/KcPageStory.ts (1)
1-1: LGTM! Type-only imports correctly applied.The inline
typespecifiers forOnInitandTypeare appropriate since these are only used as type annotations, not runtime values. This aligns with the new ESLint rule@typescript-eslint/consistent-type-imports.src/main.ts (1)
3-3: LGTM! Import reordering has no functional impact.The reordering of the
bootstrapKcApplicationimport is purely cosmetic, likely due to linting/formatting rules.tsconfig.json (1)
8-8: LGTM! Compiler options align with Angular 21 and stricter type handling.The additions of
esModuleInterop: trueandverbatimModuleSyntax: trueare appropriate:
esModuleInteropimproves CommonJS module compatibilityverbatimModuleSyntaxenforces explicit type-only imports, consistent with the new ESLint ruleAlso applies to: 24-24
eslint.config.js (1)
6-7: LGTM! ESLint configuration properly migrated todefineConfig.The refactoring from
tseslint.config()todefineConfig()fromeslint/configaligns with the updated ESLint/TypeScript-ESLint ecosystem. The import structure is clean and correct.Also applies to: 9-9
package.json (1)
30-37: Angular 21.0.1 upgrade addresses XSRF token leakage vulnerability (CVE-2025-66035).Angular 21.0.1 is published on npm and includes a critical security patch for XSRF token leakage via protocol-relative URLs. This upgrade is recommended and addresses a real security vulnerability. All Angular packages are correctly pinned to the same patched version across lines 30-37, 49-51, and 58.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/login/KcPageStory.ts (1)
62-69: Consider minimal error handling aroundgetKcPageand async lifecycleThe new
ChangeDetectorRefinjection andmarkForCheck()call after the asyncgetKcPageresolution are appropriate for a zoneless Storybook setup. To make this a bit more robust, you could optionally:
- Add a
.catch()to log or surfacegetKcPagefailures to avoid silent/unhandled rejections.- (Optional) If this story component can be torn down quickly (e.g. fast story switching), guard updates inside the
thencallback so you don’t schedule change detection on an already-destroyed instance.Example sketch:
ngOnInit() { - getKcPage(this.kcContext.pageId).then((kcPage) => { - this.pageComponent = kcPage.PageComponent; - this.userProfileFormFieldsComponent = kcPage.UserProfileFormFieldsComponent; - this.#cd.markForCheck(); - }); + getKcPage(this.kcContext.pageId) + .then((kcPage) => { + this.pageComponent = kcPage.PageComponent; + this.userProfileFormFieldsComponent = kcPage.UserProfileFormFieldsComponent; + this.#cd.markForCheck(); + }) + .catch((err) => { + // eslint-disable-next-line no-console + console.error('Failed to load kc page in story', err); + }); }Please verify that Storybook doesn’t already have a global error handler for rejected promises in this setup; if it does, you might decide this extra
.catchis unnecessary noise here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
.storybook/preview.ts(0 hunks)angular.json(2 hunks)package.json(2 hunks)src/login/KcPageStory.ts(2 hunks)
💤 Files with no reviewable changes (1)
- .storybook/preview.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧬 Code graph analysis (1)
src/login/KcPageStory.ts (1)
src/login/KcPage.ts (1)
getKcPage(11-23)
🔇 Additional comments (3)
angular.json (2)
66-74: Consistency confirmed across Storybook targets.The
experimentalZoneless: trueflag is correctly applied to both thestorybookandbuild-storybooktargets, ensuring consistent behavior during development and production builds. This aligns with the related removal ofzone.jsfrom.storybook/preview.tsmentioned in the PR changes.
57-64: The@analogjs/storybook-angularbuilder (v2.1.1) explicitly supports theexperimentalZonelessoption in Angular 21. AnalogJS documentation acknowledges that zoneless change detection is the default for Angular v21, and this configuration aligns with the project's existing use ofprovideZonelessChangeDetection()insrc/app.config.ts. No breaking changes or runtime issues are anticipated.src/login/KcPageStory.ts (1)
1-9: Imports and DI setup for Angular 21 look consistentUsing
injectforChangeDetectorRefand type-only imports forOnInit/Typematches modern Angular patterns and aligns with how this file already usesinjectforKC_LOGIN_CONTEXT; the newgetKcPage/getI18nimports are wired correctly into the story configuration.Please just confirm this file is compiled with the same Angular/TypeScript config as the rest of the app (no legacy View Engine/ES5 targets), since private fields and
typeimports rely on that toolchain support.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
package.json(2 hunks)
🔇 Additional comments (2)
package.json (2)
45-71: These devDependency upgrades align with Angular 21 defaults and require no remediation.Vitest 4.0.15 and Vite 7.3.0 are the default testing framework and build system for Angular 21. The Storybook ecosystem (10.1.9) is compatible with these versions. No build, test, or lint pipeline compatibility issues are expected.
30-37: Verify @keycloakify/angular version and test runner migration requirements.The Angular 21 upgrade to 21.0.5 (runtime) and 21.0.3 (build tools) is consistent and follows best practices. However, @keycloakify/angular's latest published version is 20.1.11, and the version 21.0.0-rc.3 mentioned in package.json cannot be verified in the npm registry—confirm whether this RC version is correct or if a different Keycloak library is intended.
Additionally, ensure the codebase has migrated from Karma to Vitest, as Angular 21 has officially swapped Karma for Vitest as the default test runner. Run
ng generate @angular/core:karma-to-vitestif needed. Verify compatibility of all upgraded devDependencies with Angular 21 and test the application thoroughly.
package.json
Outdated
| "@angular/platform-browser": "^21.0.5", | ||
| "@angular/platform-browser-dynamic": "^21.0.5", | ||
| "@angular/router": "^21.0.5", | ||
| "@keycloakify/angular": "^21.0.0-rc.3", |
There was a problem hiding this comment.
Replace with latest stable version: @keycloakify/angular 20.1.11 does not exist as version 21.0.0-rc.3.
The latest published version of @keycloakify/angular is 20.1.11. Version 21.0.0-rc.3 does not exist in the npm registry and will fail to install. Update to the latest available version.
🤖 Prompt for AI Agents
In package.json around line 38, the dependency "@keycloakify/angular":
"^21.0.0-rc.3" references a non-existent release and will fail to install;
update the version to the latest published stable release (e.g., "^20.1.11") in
the dependencies section so npm/yarn can resolve it, then run your package
manager install and verify no other packages require the removed RC version.
| "rxjs": "~7.8.2", | ||
| "tslib": "^2.8.1", | ||
| "zone.js": "~0.15.1" | ||
| "zone.js": "~0.16.0" |
There was a problem hiding this comment.
zone.js is optional in Angular 21; consider migrating to zoneless or document the requirement.
zone.js is no longer included by default in Angular 21 applications. The presence of zone.js ~0.16.0 in package.json indicates this project is either maintaining backward compatibility or hasn't migrated yet. If the application relies on zone.js, explicitly add provideZoneChangeDetection() in the root providers, and a migration will automatically add it when updating to Angular v21. If zone.js is intentional, configure it in app.config.ts and ensure zone.js is listed under polyfills in angular.json.
🤖 Prompt for AI Agents
In package.json around line 42, zone.js (~0.16.0) is present but Angular 21 no
longer includes it by default; either remove zone.js and migrate to zoneless
change detection or explicitly document and configure it: if the app relies on
zone.js, add provideZoneChangeDetection() to the root providers (so migration
will detect it), ensure zone.js is configured in app.config.ts, and list zone.js
under the polyfills array in angular.json; otherwise remove the dependency and
update any code relying on NgZone/zone-based change detection to use Angular’s
zoneless APIs.
package.json
Outdated
| "vite-tsconfig-paths": "^5.1.4", | ||
| "vitest": "^4.0.9" | ||
| "typescript-eslint": "^8.50.0", | ||
| "vite": "^7.3.0", |
There was a problem hiding this comment.
Update vite version in resolutions to match devDependencies.
There's a version mismatch: devDependencies specifies vite ^7.3.0 (line 69) while resolutions pins it to ^7.0.6 (line 75). The resolutions field will take precedence, so you'll actually be using 7.0.x instead of 7.3.x.
Apply this diff to align the versions:
"resolutions": {
"esbuild": "^0.25.4",
- "vite": "^7.0.6"
+ "vite": "^7.3.0"
},Also applies to: 75-75
🤖 Prompt for AI Agents
In package.json around lines 69 and 75 update the vite version in the
resolutions section to match devDependencies (devDependencies has "vite":
"^7.3.0" on line 69 but resolutions pins "^7.0.6" on line 75); change the
resolutions entry for vite to "^7.3.0" (or make both references use the same
exact version spec you want), so the resolutions field no longer overrides
devDependencies with a mismatched 7.0.x version.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.