refactor: remove unused imports, NgClass cleanup#43
Conversation
WalkthroughThis PR refactors the codebase to improve tree-shaking and type safety by converting regular type imports to TypeScript type-only imports, replacing NgClass directive usage with direct class bindings, removing unused directives and pipes, and adding generic type parameters to dependency injection calls for stricter type narrowing. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes The changes are highly homogeneous and repetitive across the codebase—primarily import syntax conversions, directive removals, and template binding replacements. Each modification follows a consistent pattern with no complex logic changes, branching conditions, or state management alterations. The scope spans multiple files, but the uniform nature of changes (template binding swaps, type-only imports, decorator cleanup) reduces cognitive load per file. Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
src/account/i18n/i18n.ts(1 hunks)src/account/pages/account/account.component.html(4 hunks)src/account/pages/account/account.component.ts(1 hunks)src/account/pages/applications/applications.component.ts(2 hunks)src/account/pages/federatedIdentity/federatedIdentity.component.ts(0 hunks)src/account/pages/log/log.component.ts(2 hunks)src/account/pages/password/password.component.ts(1 hunks)src/account/pages/sessions/sessions.component.ts(1 hunks)src/account/pages/totp/totp.component.ts(1 hunks)src/account/providers/keycloakify-angular/keycloakify-angular.providers.ts(2 hunks)src/account/template/template.component.html(1 hunks)src/account/template/template.component.ts(1 hunks)src/bin/initialize-account-theme/boilerplate/KcPageStory.ts(2 hunks)src/login/components/add-remove-buttons-multi-valued-attribute/add-remove-buttons-multi-valued-attribute.component.ts(1 hunks)src/login/components/field-errors/field-errors.component.ts(1 hunks)src/login/components/group-label/group-label.component.ts(1 hunks)src/login/components/input-field-by-type/input-field-by-type.component.ts(1 hunks)src/login/components/input-tag-selects/input-tag-selects.component.ts(1 hunks)src/login/components/input-tag/input-tag.component.ts(1 hunks)src/login/components/password-wrapper/password-wrapper.component.ts(1 hunks)src/login/components/select-tag/select-tag.component.ts(1 hunks)src/login/components/textarea-tag/textarea-tag.component.ts(1 hunks)src/login/i18n/i18n.ts(1 hunks)src/login/pages/login-idp-link-email/login-idp-link-email.component.ts(0 hunks)src/login/pages/login-page-expired/login-page-expired.component.ts(0 hunks)src/login/pages/login-passkeys-conditional-authenticate/login-passkeys-conditional-authenticate.component.ts(1 hunks)src/login/pages/login-username/login-username.component.ts(1 hunks)src/login/pages/login-verify-email/login-verify-email.component.ts(0 hunks)src/login/pages/login/login.component.ts(2 hunks)src/login/pages/saml-post-form/saml-post-form.component.ts(0 hunks)src/login/pages/select-authenticator/select-authenticator.component.ts(1 hunks)src/login/pages/webauthn-authenticate/webauthn-authenticate.component.ts(2 hunks)src/login/pages/webauthn-error/webauthn-error.component.ts(1 hunks)src/login/pages/webauthn-register/webauthn-register.component.ts(1 hunks)src/login/providers/keycloakify-angular/keycloakify-angular.providers.ts(1 hunks)src/login/services/login-resource-injector/login-resource-injector.service.ts(1 hunks)stories/account/KcPageStory.ts(2 hunks)stories/account/pages/KcPageStory.ts(2 hunks)
💤 Files with no reviewable changes (5)
- src/login/pages/saml-post-form/saml-post-form.component.ts
- src/account/pages/federatedIdentity/federatedIdentity.component.ts
- src/login/pages/login-idp-link-email/login-idp-link-email.component.ts
- src/login/pages/login-verify-email/login-verify-email.component.ts
- src/login/pages/login-page-expired/login-page-expired.component.ts
🧰 Additional context used
🧬 Code graph analysis (1)
stories/account/KcPageStory.ts (2)
stories/account/KcContext.ts (1)
KcContext(11-11)stories/login/KcContext.ts (1)
KcContext(11-11)
🔇 Additional comments (35)
src/login/components/textarea-tag/textarea-tag.component.ts (1)
6-8: Type-only import change looks goodLine 6 and Line 8 now correctly use
import type, avoiding runtime bundles for pure typings and aligning with Angular 20/TS best practices.src/login/components/group-label/group-label.component.ts (1)
8-8: LGTM! Type-only import improves tree-shaking.Converting to
import typeis correct sinceAttributeis only used as a type annotation. This enables the TypeScript compiler to elide the import from the emitted JavaScript, improving bundle size and tree-shaking efficiency.src/login/components/password-wrapper/password-wrapper.component.ts (1)
20-20: LGTM! AsyncPipe removal aligns with signals-based approach.The removal of
AsyncPipefrom the imports is appropriate since the component uses signals (WritableSignal) for state management rather than observables. This reduces the runtime dependency surface without affecting functionality and aligns with modern Angular v20 patterns.src/login/pages/login/login.component.ts (1)
19-19: Import cleanup looks good.Pulling in
CommonModulekeeps the template directives/pipes available after dropping the individualAsyncPipe/NgClassimports. No issues spotted.src/login/pages/webauthn-error/webauthn-error.component.ts (1)
13-13: Confirm removal of unused import: no<kc-logout-other-sessions>selector found in webauthn-error.component.html, so removingLogoutOtherSessionsComponentis safe.src/login/pages/select-authenticator/select-authenticator.component.ts (1)
13-13: LGTM – select-authenticator.component.html has no[ngClass]usage; removing the NgClass import is safe.src/account/pages/totp/totp.component.ts (1)
14-14: LGTM! NgClass removal is consistent with the refactor.The removal of NgClass from the imports array aligns with the PR objectives to simplify class bindings and reduce the directive surface. KcClassDirective and KcSanitizePipe remain as the necessary dependencies for this component.
src/account/pages/password/password.component.ts (1)
13-13: LGTM! Consistent NgClass removal.The removal of NgClass from the component's imports is consistent with the broader refactor across account pages, simplifying the dependency graph while retaining KcClassDirective for class management.
src/login/components/input-field-by-type/input-field-by-type.component.ts (1)
12-12: LGTM! Improved tree-shaking with type-only import.Converting to
import type { Attribute }correctly eliminates the runtime import sinceAttributeis only used as a type annotation (lines 35, 42-43). This change improves tree-shaking capabilities and aligns with TypeScript best practices.src/login/components/input-tag/input-tag.component.ts (1)
15-15: LGTM! Type-only import for better tree-shaking.The conversion to
import type { Attribute }is correct sinceAttributeis used exclusively as a type annotation (lines 47, 42). This improves tree-shaking and follows the pattern established in other login components.src/account/pages/sessions/sessions.component.ts (1)
13-13: LGTM! Consistent with the NgClass cleanup.The removal of NgClass from the imports follows the same pattern as other account pages, reducing the directive surface while maintaining KcClassDirective for class management.
src/account/pages/applications/applications.component.ts (1)
1-1: LGTM! Selective import cleanup.The removal of NgClass while retaining KeyValuePipe and IsArrayWithEmptyObjectPipe demonstrates selective cleanup of unused imports. This approach correctly targets only the unused NgClass directive while preserving the pipes that are actively used in the component.
Also applies to: 15-15
src/login/providers/keycloakify-angular/keycloakify-angular.providers.ts (1)
15-15: LGTM! Type-only import for provider configuration.Converting to
import type { KcContextLike }is correct sinceKcContextLikeis used exclusively as a type constraint in the configuration interface (line 23). This improves tree-shaking for the provider configuration module.src/account/pages/log/log.component.ts (1)
1-1: LGTM! Selective removal of unused NgClass.The removal of NgClass while retaining DatePipe correctly targets only the unused directive. This completes the consistent pattern of NgClass cleanup across all account page components.
Also applies to: 14-14
src/login/components/input-tag-selects/input-tag-selects.component.ts (1)
5-5: LGTM! Type-only imports improve tree-shaking.Converting these imports to type-only form is correct since
FormAction,FormFieldError, andAttributeare only used in type positions. This ensures they won't appear in the runtime bundle.Also applies to: 8-8
src/login/components/field-errors/field-errors.component.ts (1)
5-5: LGTM! Consistent type-only import pattern.The conversion to type-only imports is appropriate and aligns with the pattern established across other login components.
Also applies to: 7-7
src/login/components/add-remove-buttons-multi-valued-attribute/add-remove-buttons-multi-valued-attribute.component.ts (1)
4-4: LGTM! Appropriate use of type-only import.The
Attributetype is correctly imported as type-only since it's used solely in the type position of theattributeinput.src/login/i18n/i18n.ts (1)
2-2: LGTM! Type-only import for type alias.Correctly converted to type-only import since
MessageKey_defaultSetis used exclusively in the type alias definition.src/account/i18n/i18n.ts (1)
2-2: LGTM! Consistent pattern with login i18n.This change mirrors the type-only import pattern applied in
src/login/i18n/i18n.ts.src/account/providers/keycloakify-angular/keycloakify-angular.providers.ts (2)
14-14: LGTM! Type-only import for interface.
KcContextLikeis correctly imported as type-only since it's used only in thegetI18nfunction signature.
42-42: LGTM! Generic type parameter improves type narrowing.Using
inject<KcContext>(KC_ACCOUNT_CONTEXT)provides stricter type narrowing compared to the annotation-style approach and is the preferred Angular pattern.src/login/components/select-tag/select-tag.component.ts (1)
6-6: LGTM! Consistent type-only imports.The conversion to type-only imports follows the established pattern across login components.
Also applies to: 9-9
src/account/template/template.component.ts (1)
34-34: LGTM! Removal aligns with NgClass cleanup.Removing
KcClassDirectivefrom the component's imports is appropriate if the template has been updated to use direct class bindings instead. Based on the PR summary, the corresponding template changes have been made.src/account/pages/account/account.component.ts (1)
14-14: LGTM! Improved tree-shaking by removing NgClass.The removal of
NgClassfrom imports aligns with the template's migration to[class.has-error]bindings, reducing the bundle size through better tree-shaking.src/account/pages/account/account.component.html (1)
30-30: LGTM! Modern Angular class binding pattern.The migration from
[ngClass]="{ 'has-error': ... }"to[class.has-error]="..."is the recommended Angular approach for single-class conditionals, providing better performance and readability.Also applies to: 59-59, 85-85, 110-110
stories/account/KcPageStory.ts (2)
3-3: LGTM! Type-only import improves tree-shaking.The conversion to
import typeensuresKcContextis erased at runtime, resulting in a smaller bundle.
37-37: LGTM! Generic type parameter improves type safety.Adding the generic type parameter
inject<KcContext>(...)provides explicit type narrowing without relying on type inference, making the code more maintainable and catching type errors earlier.stories/account/pages/KcPageStory.ts (1)
3-3: LGTM! Consistent refactoring pattern.The type-only import and generic
inject<KcContext>()follow the same improvement pattern applied across the codebase, providing better tree-shaking and type safety.Also applies to: 58-58
src/account/template/template.component.html (1)
69-69: LGTM! Appropriate use of class binding patterns.The migration to
[class.active]for single-class conditionals is correct. Note that line 106 appropriately retains[ngClass]for the dynamic multi-class binding['alert', 'alert-' + message.type], demonstrating good judgment on when each pattern is appropriate.Also applies to: 73-73, 77-77, 81-81, 85-85, 88-88, 92-92, 97-97
src/bin/initialize-account-theme/boilerplate/KcPageStory.ts (1)
3-3: LGTM! Boilerplate reflects best practices.These improvements in the boilerplate file ensure that newly initialized projects start with the latest best practices for tree-shaking and type safety.
Also applies to: 58-58
src/login/pages/login-passkeys-conditional-authenticate/login-passkeys-conditional-authenticate.component.ts (1)
2-2: LGTM! Type-only import improves tree-shaking.The conversion to
import typesyntax correctly signals thatScriptis used only as a type annotation, enabling better tree-shaking by the bundler.src/login/pages/webauthn-register/webauthn-register.component.ts (1)
2-2: LGTM! Type-only import improves tree-shaking.The conversion to
import typesyntax correctly signals thatScriptis used only as a type annotation, enabling better tree-shaking by the bundler.src/login/services/login-resource-injector/login-resource-injector.service.ts (1)
6-6: LGTM! Type-only import improves tree-shaking.The conversion to
import typesyntax correctly signals thatScriptis used only as a type annotation, enabling better tree-shaking by the bundler.src/login/pages/webauthn-authenticate/webauthn-authenticate.component.ts (2)
2-2: LGTM! Type-only import improves tree-shaking.The conversion to
import typesyntax correctly signals thatScriptis used only as a type annotation, enabling better tree-shaking by the bundler.
15-15: Confirm removal of LogoutOtherSessionsComponent is correct
Nokc-logout-other-sessionsselector inwebauthn-authenticate.component.htmland no import in the component — removal is safe.
| templateUrl: 'login-username.component.html', | ||
| changeDetection: ChangeDetectionStrategy.OnPush, | ||
| imports: [KcClassDirective, AsyncPipe, KcSanitizePipe, NgClass], | ||
| imports: [KcClassDirective, KcSanitizePipe], |
There was a problem hiding this comment.
🧩 Analysis chain
LGTM! Verify the template doesn't reference removed imports.
The removal of AsyncPipe and NgClass aligns with the PR objectives. The component uses Signals (line 42) instead of Observables, making AsyncPipe unnecessary. The custom KcClassDirective appears to handle class bindings.
Run the following script to confirm the template doesn't use the removed imports:
🏁 Script executed:
#!/bin/bash
# Description: Verify that the template doesn't use async pipe or ngClass directive.
# Check for async pipe usage in the template
echo "Checking for async pipe usage..."
if fd -e html "login-username.component.html" --exec grep -n "| async" {}; then
echo "WARNING: Found async pipe usage in template"
else
echo "✓ No async pipe usage found"
fi
echo ""
# Check for ngClass directive usage in the template
echo "Checking for ngClass directive usage..."
if fd -e html "login-username.component.html" --exec grep -nE '\[ngClass\]|ngClass=' {}; then
echo "WARNING: Found ngClass directive usage in template"
else
echo "✓ No ngClass directive usage found"
fiLength of output: 272
🏁 Script executed:
#!/bin/bash
# Verify the component's imports and see what changed
echo "Component imports at line 17:"
grep -A 2 "imports:" src/login/pages/login-username/login-username.component.ts | head -5
echo ""
echo "Searching for KcClassDirective definition..."
fd -e ts -e js --exec grep -l "KcClassDirective" {} \; 2>/dev/null | head -5
echo ""
echo "Checking full component decorator:"
sed -n '/@Component/,/^}\s*$/p' src/login/pages/login-username/login-username.component.ts | head -30Length of output: 1807
🏁 Script executed:
#!/bin/bash
# Check if NgClass is required for [ngClass] bindings in Angular
# (This is general Angular knowledge that doesn't require repo access)
echo "Angular's [ngClass] directive requires NgClass import from @angular/common"
echo "ngClass bindings will fail at runtime if NgClass is not imported."Length of output: 140
🏁 Script executed:
#!/bin/bash
# Verify the component's imports to see if NgClass was removed
echo "=== Component decorator section ==="
sed -n '/@Component/,/^}\s*$/p' src/login/pages/login-username/login-username.component.ts
echo ""
echo "=== Looking for KcClassDirective definition ==="
fd -e ts --exec grep -l "directive\|export.*class.*KcClassDirective" {} \; 2>/dev/null | head -3
echo ""
echo "=== Checking template file for full ngClass context ==="
sed -n '50,65p' src/login/pages/login-username/login-username.component.htmlLength of output: 2428
Re-add NgClass to component imports
Template uses [ngClass] bindings (lines 54, 59); import NgClass from @angular/common in the component’s imports array (e.g. imports: [NgClass, KcClassDirective, KcSanitizePipe]).
🤖 Prompt for AI Agents
In src/login/pages/login-username/login-username.component.ts around line 17,
the component uses [ngClass] in the template but NgClass was not included in the
component's imports array; add NgClass imported from @angular/common to the
imports array alongside KcClassDirective and KcSanitizePipe (e.g. imports:
[NgClass, KcClassDirective, KcSanitizePipe]) and ensure the top-of-file import
statement imports NgClass from '@angular/common'.
|
Thanks for cleaning up and keeping the syntax updated! |
2 participants
Summary by CodeRabbit
Refactor
Style