Feat/angular 21 #5
Conversation
|
Visit the preview URL for this PR (updated for commit f1012aa): https://slidecontent-angularjs--pr5-feat-angular-21-3emmxovk.web.app (expires Sat, 24 Jan 2026 20:52:36 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: a46849f54b15ddea5d3d183b8d030d20fc154f13 |
There was a problem hiding this comment.
Pull request overview
This pull request attempts to upgrade the Angular application from version 19.1.4 to version 21.1.0, which includes updating the build system from @angular-devkit/build-angular to @angular/build, modernizing SCSS import syntax, migrating to new Angular control flow syntax, and adjusting configuration files.
Changes:
- Package dependencies updated to Angular 21.1.0 (currently non-existent version)
- Build system migrated from
@angular-devkit/build-angularto@angular/build - SCSS imports modernized from
@importto@usedirective - Angular template syntax updated from
*ngForto new@forcontrol flow - Font loading moved from SCSS to HTML
- TypeScript configuration updated to use
bundlermodule resolution - Zone change detection provider added to bootstrap configuration
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updates Angular and related packages to version 21.1.0 and TypeScript to 5.9.3 |
| tsconfig.json | Changes module resolution to "bundler" and removes explicit lib configuration |
| src/styles.scss | Modernizes SCSS syntax by replacing @import with @use and moves Google Fonts to HTML |
| src/index.html | Adds Google Fonts link tag to HTML head |
| src/main.ts | Adds provideZoneChangeDetection to bootstrap configuration |
| src/app/services/language.service.ts | Consolidates imports from @angular/core |
| karma.conf.js | Removes @angular-devkit/build-angular karma plugin reference |
| angular.json | Updates all builders from @angular-devkit/build-angular to @angular/build and adds global schematics configuration |
| src/app/views/home/home.component.scss | Replaces @import with @use for responsive mixins |
| src/app/views/about/about.component.scss | Replaces @import with @use for responsive mixins |
| src/app/components/base//app-.component.scss | Replaces @import with @use for responsive mixins (4 files) |
| projects/slidecontent-angular/src/lib/slide-content/slide-content.component.html | Migrates from *ngFor to @for control flow syntax |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ], |
There was a problem hiding this comment.
The Karma plugin array is incomplete - it ends with an empty line after removing the '@angular-devkit/build-angular/plugins/karma' require statement. The frameworks array on line 7 still references '@angular-devkit/build-angular', which should likely be updated to match the new '@angular/build' package structure. Please ensure both the frameworks and plugins arrays are correctly configured for Angular 21.
| import { Inject } from '@angular/core'; | ||
|
|
There was a problem hiding this comment.
The import statement for 'Inject' is redundant since it's already imported on line 1 along with other decorators from '@angular/core'. Consider removing this duplicate import statement.
| import { Inject } from '@angular/core'; |
| } | ||
|
|
||
| platformBrowserDynamic().bootstrapModule(AppModule) | ||
| platformBrowserDynamic().bootstrapModule(AppModule, { applicationProviders: [provideZoneChangeDetection()], }) |
There was a problem hiding this comment.
The provideZoneChangeDetection() is called without any configuration options. If the intention is to use the default zone-based change detection, this call may be redundant as Angular uses zone.js by default when it's imported. Consider either removing this if default behavior is desired, or passing configuration options like { eventCoalescing: true } if specific optimizations are needed.
| } | ||
|
|
||
| platformBrowserDynamic().bootstrapModule(AppModule) | ||
| platformBrowserDynamic().bootstrapModule(AppModule, { applicationProviders: [provideZoneChangeDetection()], }) |
There was a problem hiding this comment.
There is an unnecessary trailing comma after the closing bracket. While JavaScript/TypeScript allows trailing commas in many contexts, this appears inconsistent with typical formatting and may be unintentional.
| platformBrowserDynamic().bootstrapModule(AppModule, { applicationProviders: [provideZoneChangeDetection()], }) | |
| platformBrowserDynamic().bootstrapModule(AppModule, { applicationProviders: [provideZoneChangeDetection()] }) |
| "@schematics/angular:component": { | ||
| "type": "component" | ||
| }, | ||
| "@schematics/angular:directive": { | ||
| "type": "directive" | ||
| }, | ||
| "@schematics/angular:service": { | ||
| "type": "service" | ||
| }, |
There was a problem hiding this comment.
The schematic configuration sets 'type' to the same value as the schematic name (e.g., "type": "component" for component schematics). This appears to be incorrect. The 'type' property for component schematics typically doesn't exist, and if it does, it would be for specifying the component type (e.g., standalone). Please verify this configuration is correct or remove these invalid properties.
| "@schematics/angular:component": { | |
| "type": "component" | |
| }, | |
| "@schematics/angular:directive": { | |
| "type": "directive" | |
| }, | |
| "@schematics/angular:service": { | |
| "type": "service" | |
| }, | |
| "@schematics/angular:component": {}, | |
| "@schematics/angular:directive": {}, | |
| "@schematics/angular:service": {}, |
| </div> | ||
| </div> | ||
| <div class="slide-bullets"> | ||
| @for (slide of slides; track slide; let i = $index) { |
There was a problem hiding this comment.
The @for loop uses 'track slide' which tracks by object identity. For HTMLElement arrays, consider using a more stable tracking expression like 'track $index' or a unique identifier if available, to ensure proper change detection and avoid unnecessary re-rendering when the slides array is reassigned.
| @for (slide of slides; track slide; let i = $index) { | |
| @for (slide of slides; track $index; let i = $index) { |
1 participant
No description provided.