-
Notifications
You must be signed in to change notification settings - Fork 1
chore: update to angular 20 #24
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
Conversation
WalkthroughUpdates Angular/Nx/tooling versions, sets TypeScript moduleResolution to "bundler" in multiple tsconfigs, changes provideServerRendering import to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dashboard as DashboardComponent
participant RxRes as rxResource
participant EventApi as EventApiService
participant Http as HttpClient
participant API as /api/events
note right of Dashboard #D6EAF8: On view init
Dashboard->>RxRes: initialize(resource: stream -> getEvents)
RxRes->>EventApi: call getEvents()
EventApi->>Http: http.get("/api/events/all")
Http->>API: GET /api/events/all
API-->>Http: 200 [Event[]]
Http-->>EventApi: Event[]
EventApi-->>RxRes: Event[]
RxRes-->>Dashboard: emits events stream
note right of Dashboard #DFF0D8: Dashboard subscribes to stream updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
Deploying devswhomove with
|
| Latest commit: |
f5cdefe
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ee199a76.devswhomove.pages.dev |
| Branch Preview URL: | https://chore-try-nx-migrate-agular.devswhomove.pages.dev |
|
View your CI Pipeline Execution ↗ for commit f5cdefe
☁️ Nx Cloud last updated this comment at |
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: 3
🧹 Nitpick comments (8)
apps/frontend/src/app/core/services/theme.service.ts (1)
1-4: Nit: co‑locate PLATFORM_ID import.Minor readability: import
PLATFORM_IDtogether with other core symbols.-import { Injectable, signal, effect, inject } from '@angular/core'; +import { Injectable, signal, effect, inject, PLATFORM_ID } from '@angular/core'; - -import { PLATFORM_ID } from '@angular/core';apps/frontend/src/app/shared/services/auth-api.ts (1)
11-11: Use readonly for injected field.Tiny safety improvement.
- private http = inject(HttpClient); + private readonly http = inject(HttpClient);apps/frontend/src/app/shared/services/event-api.ts (1)
10-10: Use readonly for injected field.Same nit as above.
- private http = inject(HttpClient); + private readonly http = inject(HttpClient);package.json (3)
22-22: Align @angular/ssr patch version with the rest of Angular 20.2.xYou’re mixing 20.2.4 (most Angular pkgs) with @angular/ssr 20.2.2. Keep Angular package versions in lockstep to avoid peer mismatch and subtle tooling issues.
Apply:
- "@angular/ssr": "20.2.2", + "@angular/ssr": "20.2.4",
52-55: Consider aligning @angular/cli minor with devkit/build versionscli is ~20.0.0 while devkit/build are 20.2.2. Keeping them on the same minor reduces migration friction.
Example:
- "@angular/cli": "~20.0.0", + "@angular/cli": "20.2.2",
77-77: Node types are pinned to v18 while runtime/engines target Node 20/22Types misalignment can cause TS friction (e.g., fs/promises, WHATWG streams). Prefer @types/node matching your runtime.
Example:
- "@types/node": "18.16.9", + "@types/node": "22.5.4",nx.json (2)
106-112: Component generator: explicit type is good; consider also pinning ‘standalone’: trueAngular 20 defaults to standalone; setting it in generators ensures consistency across teams and tools.
Suggested addition:
"@nx/angular:component": { "style": "css", - "type": "component" + "type": "component", + "standalone": true },
110-124: Duping @nx/angular and @schematics/angular entries can confuse defaultsBoth sets define type metadata. Ensure your workspace default collection points to @nx/angular so generators resolve predictably.
If not already set elsewhere, add to nx.json or workspace config:
{ "cli": { "defaultCollection": "@nx/angular" } }
📜 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 (10)
apps/frontend/libs/event/create/tsconfig.lib.json(1 hunks)apps/frontend/libs/event/create/tsconfig.lib.prod.json(1 hunks)apps/frontend/src/app/app.config.server.ts(1 hunks)apps/frontend/src/app/core/services/theme.service.ts(1 hunks)apps/frontend/src/app/shared/components/navigation.component.ts(1 hunks)apps/frontend/src/app/shared/services/auth-api.ts(2 hunks)apps/frontend/src/app/shared/services/event-api.ts(2 hunks)apps/frontend/tsconfig.app.json(1 hunks)nx.json(1 hunks)package.json(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
apps/frontend/tsconfig.app.json (1)
apps/frontend/vite.config.mts (1)
root(7-27)
apps/frontend/src/app/shared/services/auth-api.ts (4)
apps/frontend/src/app/shared/types/user-profile.ts (1)
UserProfile(1-7)apps/frontend/src/app/pages/profile/profile.component.ts (1)
ProfileComponent(155-157)apps/frontend/src/app/pages/auth/auth-success/auth-success.component.ts (1)
AuthSuccessComponent(36-77)apps/frontend/src/app/guards/auth.guard.ts (2)
authApi(8-23)authApi(25-40)
apps/frontend/libs/event/create/tsconfig.lib.prod.json (1)
apps/frontend/vite.config.mts (1)
root(7-27)
apps/frontend/src/app/app.config.server.ts (2)
apps/frontend/src/server.ts (2)
app(11-50)req(34-47)apps/frontend/src/main.server.ts (1)
bootstrapApplication(5-5)
apps/frontend/src/app/shared/services/event-api.ts (2)
apps/frontend/src/app/pages/dashboard/dashboard.component.ts (2)
DashboardComponent(31-37)eventApiService(34-34)apps/frontend/src/app/pages/profile/profile.component.ts (1)
ProfileComponent(155-157)
apps/frontend/src/app/shared/components/navigation.component.ts (2)
apps/frontend/src/app/core/components/header/header.ts (2)
Component(11-50)HeaderComponent(38-50)apps/frontend/src/app/pages/landing/landing.component.ts (2)
Component(8-27)HomeComponent(23-27)
🔇 Additional comments (11)
apps/frontend/src/app/core/services/theme.service.ts (1)
3-3: Import consolidation LGTM.Merging
DOCUMENTandisPlatformBrowserunder@angular/commonis correct for Angular 20. No behavior change.apps/frontend/src/app/app.config.server.ts (2)
1-1: Correct migration to @angular/ssr.Switching
provideServerRenderingto@angular/ssrmatches current SSR API. ✔️ (angular.dev)
1-9: SSR consistency check — CommonEngine import OK
No action required: apps/frontend/src/server.ts imports CommonEngine from '@angular/ssr/node' (not '@angular/platform-server').apps/frontend/libs/event/create/tsconfig.lib.json (1)
8-10: Align bundler moduleResolution with prod and confirm declaration/publishing plan."moduleResolution": "bundler" is OK, but pair it with an appropriate "compilerOptions.module" and a clear type-declaration strategy — apps/frontend/libs/event/create/package.json contains "name": "@devswhorun/create-event" (likely a packaged lib).
- apps/frontend/libs/event/create/tsconfig.lib.json — ensure "compilerOptions.module" matches prod target and keep tsconfig options consistent across apps/libs to avoid tsserver/editor drift.
- If this package is published: enable declarations (e.g. "declaration": true or emitDeclarationOnly in build) and add "types" or include the .d.ts files in package.json "files".
- If not published: set "private": true in apps/frontend/libs/event/create/package.json or remove the "name".
apps/frontend/libs/event/create/tsconfig.lib.prod.json (1)
4-5: Library build + bundler resolution — confirm declaration outputapps/frontend/libs/event/create/project.json shows the build uses @nx/angular:ng-packagr-lite. moduleResolution: "bundler" in apps/frontend/libs/event/create/tsconfig.lib.prod.json (lines 4–5) can produce per-file .d.ts imports that break published packages. Either switch moduleResolution to "nodenext" for publishable libs or ensure declarations are bundled/flattened (emit a single .d.ts). Also confirm "module" is set to "esnext" or "preserve" — no "module" setting was found in the tsconfig files; verify.
apps/frontend/src/app/shared/components/navigation.component.ts (1)
8-8: Safe to drop CommonModule from NavigationComponent.apps/frontend/src/app/shared/components/navigation.component.ts uses only @let/@if and routerLink; there are no *ngIf/*ngFor or CommonModule pipes (date, uppercase, lowercase, slice, json, async) in this file. Ensure other components that use pipes (e.g., event-card.component.ts) retain CommonModule where required.
apps/frontend/tsconfig.app.json (1)
5-6: Bundler resolution — confirmed (base module = "esnext", TS ≥ 5.4 present).tsconfig.base.json has "module": "esnext"; apps/frontend/tsconfig.app.json uses "moduleResolution": "bundler"; root package.json devDependency "typescript" = 5.9.2 (>=5.4). Paths remain supported — nxViteTsPaths should continue to work.
package.json (2)
14-21: Angular 20 dependency set otherwise looks consistentPinned 20.2.x across core, forms, router, ng-packagr 20.2.0, rxjs ~7.8, zone.js ~0.15 are appropriate for Angular 20.
Also applies to: 37-37, 41-43
20-20: Confirm removal of @angular/platform-server dependencyRepository search for "@angular/platform-server", renderModule, or renderApplication returned no matches; if SSR migration to @angular/ssr is complete and no platform-server APIs are used, remove the dependency from package.json (current: "@angular/platform-server": "20.2.4" at line 20).
nx.json (2)
131-160: typeSeparator “.” LGTM across guard/interceptor/module/pipe/resolverThis enforces predictable names (e.g., foo.guard.ts). Good for consistency with many style guides.
76-88: Confirmed — @nx/jest@21.5.2 is compatible with Jest 30Workspace already uses jest@30.0.5 and ts-jest@29.4.1; Nx v21 includes explicit Jest 30 support and @nx/jest migrations require jest >=30 and ts-jest >=29.4. (nx.dev)
Jest's reporters support tuples [module, options] (the Jest 30 config shape); keep reporter+options in your jest.config.* because the @nx/jest executor/project
reportersoption historically accepts only string[] and can trigger validation warnings. (fossies.org)
| constructor() { | ||
| this.loadStoredSession(); | ||
| } |
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.
💡 Verification agent
🧩 Analysis chain
SSR crash risk: constructor triggers localStorage on the server.
constructor() calls loadStoredSession(), which hits localStorage. On SSR this throws (no window/localStorage). Gate the call behind an isPlatformBrowser check.
-import { Injectable, signal, inject } from '@angular/core';
+import { Injectable, signal, inject, PLATFORM_ID } from '@angular/core';
+import { isPlatformBrowser } from '@angular/common';
@@
export class AuthApiService {
- private http = inject(HttpClient);
+ private readonly http = inject(HttpClient);
+ private readonly platformId = inject(PLATFORM_ID);
@@
- constructor() {
- this.loadStoredSession();
- }
+ constructor() {
+ if (isPlatformBrowser(this.platformId)) {
+ this.loadStoredSession();
+ }
+ }🏁 Script executed:
#!/bin/bash
# Prove risk surface: any server-side code that injects AuthApiService?
rg -nC3 'inject\(\s*AuthApiService\s*\)' apps/frontend/src | sed -n '1,200p'
# Grep for direct usage of localStorage in this service
rg -n 'localStorage' apps/frontend/src/app/shared/services/auth-api.tsLength of output: 4759
SSR crash risk — AuthApiService uses localStorage during construction
constructor() calls loadStoredSession() which accesses localStorage; on SSR (no window/localStorage) this will throw.
- Immediate fix: in apps/frontend/src/app/shared/services/auth-api.ts inject PLATFORM_ID and call loadStoredSession() only when isPlatformBrowser(this.platformId).
- Also fix every direct localStorage access in that file (lines: 59, 63, 83, 123, 144–145, 164–165) — either wrap them with isPlatformBrowser(...) or extract an SSR-safe StorageService that no-ops on the server (preferred).
- Rationale: AuthApiService is injected by server-rendered code (auth.guard.ts, core/header.ts, shared/navigation.component.ts, pages/*), so the service will be instantiated during SSR and currently crashes.
| "@nestjs/common": "^10.0.2", | ||
| "@nestjs/config": "^4.0.2", | ||
| "@nestjs/core": "^10.0.2", | ||
| "@nestjs/platform-express": "^10.0.2", | ||
| "@nestjs/swagger": "^11.2.0", |
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.
💡 Verification agent
🧩 Analysis chain
Check NestJS major skew: @nestjs/swagger 11 with Nest 10
swagger@11 may expect Nest 11 peer ranges. Either bump Nest core packages to 11 or hold swagger at a compatible 10.x.
Quick check:
🏁 Script executed:
#!/bin/bash
npm view @nestjs/swagger@11 peerDependencies --json | jq .Length of output: 3330
Fix Nest/@nestjs/swagger version mismatch
package.json lists @nestjs/* at ^10.0.2 but @nestjs/swagger@11 has peerDependencies requiring @nestjs/core and @nestjs/common ^11.0.1 — either upgrade Nest packages to 11 or pin @nestjs/swagger to a 10.x release to avoid peerDependency conflicts.
File: package.json (lines 23-27)
🤖 Prompt for AI Agents
In package.json around lines 23 to 27, there is a version mismatch: @nestjs/*
are ^10.0.2 while @nestjs/swagger is ^11.2.0 which requires Nest v11; fix by
either (A) upgrade all @nestjs/* packages to compatible v11 versions (bump
@nestjs/common, @nestjs/core, @nestjs/platform-express, @nestjs/config to ^11.x
and run full install/test) or (B) pin @nestjs/swagger to a 10.x release that
matches Nest v10 (change @nestjs/swagger to a 10.x version), then run npm/yarn
install and run tests to verify no peerDependency issues remain.
| "jest": "30.0.5", | ||
| "jest-environment-node": "^29.7.0", |
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.
💡 Verification agent
🧩 Analysis chain
Jest 30 is inconsistent with ts-jest 29 and jest-environment-node 29
This combination will break tests (major mismatch). Either upgrade ts-jest and env to 30 or pin Jest to 29.
Option A (keep Jest 30):
- "ts-jest": "29.4.1",
+ "ts-jest": "30.0.0",
- "jest-environment-node": "^29.7.0",
+ "jest-environment-node": "30.0.5",Option B (downgrade Jest to 29):
- "jest": "30.0.5",
+ "jest": "29.7.0",
- "@types/jest": "30.0.0",
+ "@types/jest": "29.5.12",
- "jest-util": "30.0.5"
+ "jest-util": "29.7.0"Run to confirm peer ranges:
Also applies to: 92-92, 76-76, 100-100
🏁 Script executed:
#!/bin/bash
npm view ts-jest@latest version
npm view ts-jest@30 peerDependencies --json | jq .
npm view jest-environment-node@latest versionLength of output: 604
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== existing checks (from user) ==="
npm view ts-jest@latest version || true
npm view ts-jest@30 peerDependencies --json || true
npm view jest-environment-node@latest version || true
echo
echo "=== check candidate versions ==="
npm view jest@29.7.0 version || true
npm view "@types/jest@29.5.12" version || true
npm view jest-util@29.7.0 version || true
echo
echo "=== check 30.x candidates ==="
npm view jest@30.0.5 version || true
npm view "@types/jest@30.0.0" version || true
npm view jest-util@30.0.5 version || true
echo
echo "=== latest versions ==="
npm view jest version || true
npm view "@types/jest" version || true
npm view jest-util version || true
npm view ts-jest version || trueLength of output: 801
Downgrade Jest to 29 to match ts-jest (critical mismatch — tests will break)
ts-jest@latest is 29.4.1 and ts-jest@30 is not published (npm E404); package.json currently has jest 30.x while jest-environment-node is ^29.7.0 — pin Jest and related packages to 29.x.
File: package.json — lines: 86-87 (also check 76, 92, 100)
- "jest": "30.0.5",
+ "jest": "29.7.0",
- "@types/jest": "30.0.0",
+ "@types/jest": "29.5.12",
- "jest-util": "30.0.5"
+ "jest-util": "29.7.0"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "jest": "30.0.5", | |
| "jest-environment-node": "^29.7.0", | |
| "jest": "29.7.0", | |
| "jest-environment-node": "^29.7.0", |
🤖 Prompt for AI Agents
In package.json around lines 86-87 (also review lines 76, 92, 100), Jest is
pinned to v30 while related packages (jest-environment-node and ts-jest) are on
the 29.x line, creating a critical mismatch; update package.json to pin jest to
a 29.x version compatible with ts-jest (e.g., "^29.7.0" or the exact 29 version
your other deps use), ensure jest-environment-node and ts-jest are aligned to
the same 29.x range (for ts-jest use the published 29.4.1 if required), and
update any other Jest-related entries at the noted lines so all Jest-related
packages use the same major version 29 to prevent test failures.
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)
apps/frontend/src/app/pages/dashboard/dashboard.component.ts (2)
33-36: Type clarity nit: annotate defaultValueAvoid [] inferring to never[] in edge cases; annotate for clarity.
- events = rxResource({ - stream: () => this.eventApiService.getEvents(), - defaultValue: [], - }); + events = rxResource({ + stream: () => this.eventApiService.getEvents(), + defaultValue: [] as Event[], + });
22-24: Optional UX: add retry with events.reload() and show errorSmall improvement to help users recover from transient failures. (angular.jp)
- <div>Error loading events</div> + <div class="space-x-2"> + <span>Error loading events</span> + <button class="btn btn-sm" (click)="events.reload()">Retry</button> + @if (events.error()) { <span class="text-red-600">{{ events.error()?.message }}</span> } + </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/frontend/src/app/pages/dashboard/dashboard.component.ts(1 hunks)
⏰ 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). (1)
- GitHub Check: main
🔇 Additional comments (1)
apps/frontend/src/app/pages/dashboard/dashboard.component.ts (1)
33-36: Correct migration — rxResource uses stream (Angular v20); approve but verify no leftoverloader:usageSwitching from
loader→streamis correct; returning Observable<Event[]> and usingdefaultValueto keep the template non-null is appropriate. File checked: apps/frontend/src/app/pages/dashboard/dashboard.component.ts (lines 33–36).Automated search here was inconclusive. Run these locally to confirm no remaining
rxResourceusages still usingloader::# list files using rxResource rg -n --hidden --glob '!**/node_modules/**' 'rxResource' -S || true # search for any 'loader:' occurrences rg -n --hidden --glob '!**/node_modules/**' -S -P 'loader\s*:' || true
This pull request primarily upgrades the project from Angular 19 to Angular 20, along with associated Nx, Jest, and tooling dependencies. It also updates TypeScript configuration files to use the new
bundlermodule resolution strategy, refines some import statements for compatibility, and improves Nx schematic configuration for better code generation.Framework and Tooling Upgrades:
@angular/core,@angular/ssr, and others) to version 20.x inpackage.json, along with Nx, Jest, and related build tools. This ensures compatibility with the latest Angular features and ecosystem improvements. [1] [2] [3]TypeScript Configuration Updates:
"moduleResolution": "bundler"to multiple TypeScript config files (tsconfig.app.json,tsconfig.lib.json,tsconfig.lib.prod.json) to align with Angular 20's recommended module resolution strategy. [1] [2] [3]Code Compatibility Adjustments:
theme.service.tsandapp.config.server.tsto match Angular 20's API changes, such as importingDOCUMENTdirectly from@angular/coreand using@angular/ssrfor server rendering. [1] [2]Component Module Imports:
CommonModulefrom theimportsarray innavigation.component.tssince it's no longer required for this component, simplifying the module imports.Nx Schematic Configuration Improvements:
nx.jsonwith explicit schematic type and separator settings for components, directives, services, and other Angular entities to improve code generation consistency and clarity.Summary by CodeRabbit
New Features
Refactor
Chores