-
Notifications
You must be signed in to change notification settings - Fork 0
✨ start upgrading to angular 20 #3
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughAngular dependencies were upgraded to version 20.1.0 in main and package-specific Changes
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 1
🧹 Nitpick comments (2)
angular-code-input/package.json (1)
4-4
: Update description to reflect supported Angular version
The description still says “Angular 18+ projects”. After bumping peer deps to 19, update marketing copy to avoid confusion.- "description": "Code or pin code input for Angular 18+ projects", + "description": "Code or pin code input for Angular 19+ projects",angular-code-input/src/lib/code-input.component.ts (1)
31-35
:standalone: false
is redundant
standalone
defaults tofalse
for NgModules-based components; adding it increases noise and may confuse consumers who are migrating to true standalone APIs. Recommend dropping the property unless tooling explicitly requires it.- styleUrls: ['./code-input.component.scss'], - standalone: false + styleUrls: ['./code-input.component.scss']
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (3)
angular-code-input/package.json
(1 hunks)angular-code-input/src/lib/code-input.component.ts
(1 hunks)package.json
(2 hunks)
🔇 Additional comments (2)
package.json (1)
23-33
: Angular versions upgraded only to 19-series despite PR title referencing 20
All framework deps are pinned to^19.2.x
, which contradicts the PR goal “start upgrading to angular 20”. If 20 .x is really the target, these ranges will block it (the caret stops at<20.0.0
). Please confirm whether 19.x is an intermediate step or bump to^20.0.0
/~20.0.0-rc
as required.angular-code-input/package.json (1)
26-28
: Peer-dependency floor looks good but upper range is unbounded
Using">=19.2.14"
allows any future major (21, 22…) which might introduce breaking changes for libraries. Consider the more typical caret range (^19.0.0 || ^20.0.0-0
) or add<21
until you validate against v21.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (6)
angular-code-input/package.json
(1 hunks)angular-code-input/src/lib/code-input.component.html
(1 hunks)angular-code-input/tsconfig.lib.json
(1 hunks)angular.json
(1 hunks)package.json
(2 hunks)tsconfig.json
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- tsconfig.json
- angular.json
🚧 Files skipped from review as they are similar to previous changes (2)
- angular-code-input/package.json
- package.json
🔇 Additional comments (2)
angular-code-input/tsconfig.lib.json (1)
6-6
: Switching tomoduleResolution: "bundler"
may break downstream buildsLibrary bundles produced by
ng-packagr
still rely on classic Node-style module resolution when generating the.d.ts
and FESM bundles. Setting"bundler"
at library level can:• Cause path mapping / type-reference look-ups to fail during
ng build <lib>
• Trip consumers that compile with plaintsc
(they inherit your emittedtsconfig.metadata.json
)Unless you have verified the full publish-and-consume flow, keep
"node"
here and only use"bundler"
in the applicationtsconfig
:- "moduleResolution": "bundler", + "moduleResolution": "node",Run
npm run build && npm pack
and consume the tarball in a fresh project to confirm.angular-code-input/src/lib/code-input.component.html (1)
1-15
: Template control-flow block misses a trailing semicolonAngular’s control-flow grammar expects a semicolon before the closing parenthesis:
@for (item of items; track item; let i = $index; ) { … }
Most examples omit it only for brevity, but the compiler in strict mode (
fullTemplateTypeCheck
) will complain. Verify the template still compiles; if not, append the semicolon:-@for (holder of placeholders; track holder; let i = $index) { +@for (holder of placeholders; track holder; let i = $index; ) {
@for (holder of placeholders; track holder; let i = $index) { | ||
<span | ||
[class.code-hidden]="isCodeHidden"> | ||
<input #input | ||
(click)="onClick($event)" | ||
(paste)="onPaste($event, i)" | ||
(input)="onInput($event, i)" | ||
(keydown)="onKeydown($event, i)" | ||
[type]="inputType" | ||
[disabled]="disabled" | ||
[attr.inputmode]="inputMode" | ||
[attr.autocapitalize]="autocapitalize" | ||
autocomplete="one-time-code"/> | ||
</span> | ||
} |
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.
🛠️ Refactor suggestion
Use a stable key in track
to avoid unnecessary DOM churn
track holder
re-evaluates by object identity.
If placeholders
is rebuilt on every change-detection cycle (e.g. Array(n).fill('')
), all inputs will be re-created. Prefer a stable scalar such as the loop index:
-@for (holder of placeholders; track holder; let i = $index) {
+@for (holder of placeholders; track i; let i = $index) {
This keeps focus/selection intact and improves performance.
📝 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.
@for (holder of placeholders; track holder; let i = $index) { | |
<span | |
[class.code-hidden]="isCodeHidden"> | |
<input #input | |
(click)="onClick($event)" | |
(paste)="onPaste($event, i)" | |
(input)="onInput($event, i)" | |
(keydown)="onKeydown($event, i)" | |
[type]="inputType" | |
[disabled]="disabled" | |
[attr.inputmode]="inputMode" | |
[attr.autocapitalize]="autocapitalize" | |
autocomplete="one-time-code"/> | |
</span> | |
} | |
@for (holder of placeholders; track i; let i = $index) { | |
<span | |
[class.code-hidden]="isCodeHidden"> | |
<input #input | |
(click)="onClick($event)" | |
(paste)="onPaste($event, i)" | |
(input)="onInput($event, i)" | |
(keydown)="onKeydown($event, i)" | |
[type]="inputType" | |
[disabled]="disabled" | |
[attr.inputmode]="inputMode" | |
[attr.autocapitalize]="autocapitalize" | |
autocomplete="one-time-code"/> | |
</span> | |
} |
🤖 Prompt for AI Agents
In angular-code-input/src/lib/code-input.component.html lines 1 to 15, the
*ngFor trackBy uses 'holder' which tracks by object identity causing unnecessary
DOM re-creation if placeholders array is rebuilt. Change the trackBy expression
to use the loop index 'i' instead of 'holder' to provide a stable scalar key,
preventing input elements from being recreated and preserving focus and
selection.
… and to have it own provider
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: 1
🔭 Outside diff range comments (1)
angular-code-input/README.md (1)
1-264
: Unify and Sync Documentation FilesThe two README files (
README.md
andangular-code-input/README.md
) are almost identical but have diverged slightly. This duplication risks getting out of sync. Please choose one source of truth or keep both in lockstep by:
Removing one of the files if it’s not needed.
Or, if both are required (e.g., one lives in the package folder), updating
angular-code-input/README.md
to match the root copy:• Re-add the missing
<b>inputMode</b>
entry in the Component options table (present inREADME.md
around line 213).
• Resolve minor formatting inconsistencies around the closing</code-input>
tags and sample method definitions so they match exactly.Once synced, future edits only need to be made in one place, avoiding documentation drift.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md
(1 hunks)angular-code-input/README.md
(1 hunks)angular-code-input/src/lib/code-input.component.config.ts
(2 hunks)angular-code-input/src/lib/code-input.component.ts
(4 hunks)angular-code-input/src/lib/code-input.module.ts
(0 hunks)angular-code-input/src/public-api.ts
(0 hunks)
💤 Files with no reviewable changes (2)
- angular-code-input/src/public-api.ts
- angular-code-input/src/lib/code-input.module.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
angular-code-input/src/lib/code-input.component.ts (1)
angular-code-input/src/lib/code-input.component.config.ts (2)
CodeInputComponentConfigToken
(3-3)defaultComponentConfig
(19-31)
🪛 markdownlint-cli2 (0.17.2)
README.md
43-43: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
44-44: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
57-57: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
58-58: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
69-69: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
71-71: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
74-74: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
90-90: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
106-106: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
109-109: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
123-123: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
124-124: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
🪛 Biome (1.9.4)
angular-code-input/src/lib/code-input.component.ts
[error] 81-81: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
🔇 Additional comments (11)
angular-code-input/src/lib/code-input.component.config.ts (3)
1-1
: LGTM: Modern provider pattern implementationThe addition of the
Provider
import enables the new provider functions that follow Angular's modern dependency injection patterns.
33-43
: Excellent modernization with proper configuration mergingThe
provideCodeInputConfig
function provides a clean, type-safe way to configure the component while properly merging with default configuration using the spread operator.
45-55
: Great addition for dynamic configuration scenariosThe
provideCodeInputConfigFactory
function enables dynamic configuration based on runtime conditions (environment, services, etc.), which is essential for modern Angular applications.README.md (4)
41-67
: Excellent documentation for standalone component migrationThe clear separation between standalone components (recommended) and NgModule-based applications provides developers with appropriate guidance for their specific use cases.
69-104
: Comprehensive modern configuration examplesThe documentation effectively demonstrates the transition from legacy
.forRoot()
patterns to modern provider functions, with practical examples for both standalone apps and NgModule-based applications.
106-122
: Valuable advanced configuration patternThe factory-based configuration example showcases important real-world scenarios where dynamic configuration based on environment or other services is needed.
123-136
: Good backward compatibility documentationProperly documenting the legacy approach as "Still Supported" ensures existing users can migrate at their own pace while encouraging adoption of modern patterns.
angular-code-input/src/lib/code-input.component.ts (4)
7-7
: LGTM: Modern dependency injection importThe addition of
inject
import enables the modern Angular dependency injection pattern used later in the component.
17-17
: Good addition for standalone component supportThe
CommonModule
import is essential for standalone component functionality, particularly for common directives that might be used in the template.
30-35
: Proper standalone component configurationThe component decorator correctly includes
CommonModule
in the imports array, enabling standalone component usage as documented in the README.
69-70
: Excellent modernization to inject() functionThe migration from constructor-based dependency injection to the
inject()
function follows Angular's modern patterns and improves code readability.
for (const prop in config) { | ||
if (!config.hasOwnProperty(prop)) { | ||
for (const prop in this.config) { | ||
if (!this.config.hasOwnProperty(prop)) { |
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.
🛠️ Refactor suggestion
Modernize to Object.hasOwn() for better JavaScript practices
The static analysis correctly identifies that Object.hasOwn()
is preferred over Object.hasOwnProperty()
in modern JavaScript.
Apply this diff to use the modern approach:
- if (!this.config.hasOwnProperty(prop)) {
+ if (!Object.hasOwn(this.config, prop)) {
🧰 Tools
🪛 Biome (1.9.4)
[error] 81-81: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
🤖 Prompt for AI Agents
In angular-code-input/src/lib/code-input.component.ts at line 81, replace the
use of this.config.hasOwnProperty(prop) with Object.hasOwn(this.config, prop) to
modernize the code and follow current JavaScript best practices.
Summary by CodeRabbit