Update to angular 21 #45
Conversation
WalkthroughSwitched ESLint config to use defineConfig and added languageOptions; upgraded Angular/tooling and other deps to v21 and bumped package versions; added verbatimModuleSyntax in tsconfig and removed lib; converted many runtime imports to TypeScript type-only imports; minor reflows and a few message/CI/story removals. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/login/components/input-tag/input-tag.component.ts (1)
56-88: FixfieldIndexsignal handling and falsy index checks
fieldIndexis created viainput<number | undefined>(), sothis.fieldIndexis a signal function, and you should read it viathis.fieldIndex().- In
value,if (index)treats0as falsy, so the first element in an array is never returned.- In
onChange,if (this.fieldIndex !== undefined)always passes (the function is neverundefined); should check the signal value viafieldIndex()instead.- The template also has the same falsy check issue at line 10:
[attr.aria-invalid]="index && displayableErrors()?.[index]"should useindex != null.This causes incorrect behavior for multi-valued attributes, especially when
fieldIndexis0.Suggested fixes:
In
valuecomputed:if (index != null) { return valueOrValues[index] ?? null; }In
onChange:const fieldIndex = this.fieldIndex(); if (fieldIndex != null && valueOrValues instanceof Array) { return valueOrValues.map((value, i) => i === fieldIndex ? (event.target as HTMLInputElement)?.value : value ); }In template line 10:
[attr.aria-invalid]="index != null && displayableErrors()?.[index] !== undefined"src/account/DefaultPage/DefaultPage.ts (1)
60-65: Return type may beundefinedfor unhandledpageIdvalues.If
pageIddoesn't match any switch case,ComponentBootstrapPromiseremainsundefined, and the optional chaining?.then()returnsundefined. However, the function's return type isPromise<{...}>without accounting for this.Consider either:
- Adding a
defaultcase that throws an error for unhandled page IDs- Updating the return type to
Promise<{...}> | undefinedcase 'federatedIdentity.ftl': ComponentBootstrapPromise = import('@keycloakify/angular/account/pages/federatedIdentity').then( c => c.FederatedIdentityComponent ); break; + default: + throw new Error(`Unhandled pageId: ${pageId}`); } - return ComponentBootstrapPromise?.then(ComponentBootstrap => ({ + return ComponentBootstrapPromise.then(ComponentBootstrap => ({ ComponentBootstrap, doMakeUserConfirmPassword: false, doUseDefaultCss, classes }));
🧹 Nitpick comments (1)
src/login/pages/saml-post-form/saml-post-form.component.ts (1)
50-61: Minor style suggestion: simplify the null-coalescing check.Line 51 performs two separate null and undefined checks; you could simplify this to
if (!this.htmlFormElement())for brevity, since signal getters return a falsy value when unset.- if (this.htmlFormElement() === null || this.htmlFormElement() === undefined) { + if (!this.htmlFormElement()) { return; }
📜 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 (32)
eslint.config.js(2 hunks)package.json(2 hunks)src/account/DefaultPage/DefaultPage.ts(2 hunks)src/account/classes/component-reference/component-reference.class.ts(1 hunks)src/account/directives/kc-class/kc-class.directive.ts(1 hunks)src/account/services/account-resource-injector/account-resource-injector.service.ts(1 hunks)src/account/template/template.component.ts(2 hunks)src/eslint.config.js(1 hunks)src/lib/pipes/input-type/input-type.pipe.ts(1 hunks)src/lib/pipes/is-array-with-empty-object/is-array-with-empty-object.pipe.ts(1 hunks)src/lib/pipes/kc-sanitize/kc-sanitize.pipe.ts(1 hunks)src/lib/pipes/to-array/to-array.pipe.ts(1 hunks)src/lib/pipes/to-number/to-number.pipe.ts(1 hunks)src/lib/services/resource-injector/resource-injector.service.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/user-profile-form-fields/user-profile-form-fields.component.ts(1 hunks)src/login/defaultPage.ts(4 hunks)src/login/directives/kc-class/kc-class.directive.ts(1 hunks)src/login/pages/idp-review-user-profile/idp-review-user-profile.component.ts(1 hunks)src/login/pages/login-recovery-authn-code-config/login-recovery-authn-code-config.component.ts(1 hunks)src/login/pages/login-update-profile/login-update-profile.component.ts(1 hunks)src/login/pages/login/login.component.ts(2 hunks)src/login/pages/register/register.component.ts(1 hunks)src/login/pages/saml-post-form/saml-post-form.component.ts(1 hunks)src/login/pages/update-email/update-email.component.ts(1 hunks)src/login/services/user-profile-form/user-profile-form.service.ts(2 hunks)src/login/template/template.component.ts(2 hunks)src/package.json(1 hunks)tsconfig.json(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: test
🔇 Additional comments (35)
src/login/components/password-wrapper/password-wrapper.component.ts (1)
1-1: LGTM!The type-only import of
WritableSignalis correctly applied. SinceWritableSignalis used only as a type annotation (line 36), converting it to a type-only import reduces runtime bundle size without affecting functionality. This aligns with the PR's systematic refactoring toward type-only imports and is compatible with Angular 21 andverbatimModuleSyntax: true.src/login/components/input-tag-selects/input-tag-selects.component.ts (1)
1-1: Type-only import forSignalcorrectly applied.The
type Signalannotation on line 1 properly marks this import as type-only, which aligns with TypeScript'sverbatimModuleSyntax: truerequirement introduced in the Angular 21 upgrade. SinceSignalis used exclusively as a type annotation on line 40 and never as a runtime value, this change is correct and follows best practices.src/login/pages/saml-post-form/saml-post-form.component.ts (2)
1-10: Import refactoring aligns with Angular 21 and TypeScript strict mode.The type-only import formatting correctly marks compile-time-only types (
AfterViewInit,ElementRef,TemplateRef) while preserving runtime values. This matches the PR's shift toward stricter TypeScript settings withverbatimModuleSyntax: true.
20-49: Modern Angular patterns in place; component structure is sound.Dependency injection with
inject<T>(), theviewChild()signal API, and proper type generics all demonstrate contemporary Angular 21 patterns.src/lib/pipes/is-array-with-empty-object/is-array-with-empty-object.pipe.ts (1)
1-13: Type-only import forPipeTransformis correctThe switch to a type-only import is consistent with
verbatimModuleSyntaxand does not affect runtime behavior. Pipe logic remains unchanged and looks good.src/lib/services/resource-injector/resource-injector.service.ts (1)
1-50: Type-only imports forRenderer2andScriptlook goodConverting
Renderer2andScriptto type-only imports matches their usage (annotation-only) and is compatible with TSverbatimModuleSyntax. The observable-based injection logic is unchanged.src/login/components/input-tag/input-tag.component.ts (1)
12-12: Type-only imports forFormAction/FormFieldErrorare appropriateBoth symbols are only used as types, so marking them as type-only imports is correct and keeps the runtime import surface minimal.
src/lib/pipes/input-type/input-type.pipe.ts (1)
1-13: InputTypePipe type-only import is fineUsing a type-only import for
PipeTransformcorrectly reflects its usage and works well with stricter TS module settings. The pipe behavior (html5- prefix stripping with default'text') is unchanged and looks good.src/login/pages/login-recovery-authn-code-config/login-recovery-authn-code-config.component.ts (1)
1-171: Type-onlyAfterViewInitimport matches its usage
AfterViewInitis only referenced in theimplementsclause, so importing it as a type is appropriate and keeps the runtime import surface minimal. The lifecycle hook implementation remains unchanged.src/login/components/user-profile-form-fields/user-profile-form-fields.component.ts (1)
2-55: TemplateRef as a type-only import is appropriate
TemplateRefis only used for thebeforeField/afterFieldproperty types, so marking it as a type-only import is correct and aligns with the rest of the PR’simport typecleanup.src/login/pages/login/login.component.ts (1)
1-20: This change is correct and safe — no CommonModule directives are used in the templateThe template has been modernized to use Angular's new control flow syntax (@if, @for, @let) instead of structural directives like *ngIf and *ngFor. The only ngClass usage is the property binding
[ngClass], which is provided by the imported NgClass directive. The replacement of CommonModule with NgClass is appropriate and complete.src/package.json (1)
5-9: Angular 21 and keycloakify peer dependency versions are correctly alignedThe peerDependencies in src/package.json correctly specify keycloakify
^11.13.0and Angular 21.0.0. These versions match the root package.json devDependencies (keycloakify^11.13.0and Angular 21.0.1), which are compatible and align with the PR's Angular 21 upgrade goal. Keycloakify 11.13.0 is framework-agnostic and compatible with Angular 21.tsconfig.json (1)
25-25:verbatimModuleSyntaxaligns with your type-only imports/bundler setupEnabling
verbatimModuleSyntaxhere is consistent with the project-wide move toimport typeandmoduleResolution: "bundler". No issues from this change on its own.src/account/classes/component-reference/component-reference.class.ts (1)
1-6: Type-onlyClassKeyimport is appropriate
ClassKeyis used purely in type positions, so switching toimport { type ClassKey }is correct and keeps the runtime bundle leaner.src/login/components/input-field-by-type/input-field-by-type.component.ts (1)
10-10: ConvertingFormAction/FormFieldErrorto type-only imports is safeBoth are only used in generic type positions (
input/output), so making them type-only imports correctly removes them from the emitted JS without changing behavior.src/login/pages/idp-review-user-profile/idp-review-user-profile.component.ts (1)
2-2: Type-onlyTemplateRef/Typeimport matches their usage
TemplateRefandTypeare only referenced in generic type parameters (viewChild<TemplateRef<...>>,input<Type<...>>()), so importing them as types is correct and has no runtime effect.src/login/services/user-profile-form/user-profile-form.service.ts (1)
1-15: Type-only imports forOnDestroy,SafeHtml, andObservableare correctThese symbols are used only in type positions (
implements OnDestroy, property types, andformState$: Observable<FormState>), while runtime values (DomSanitizer,BehaviorSubject,map) remain value imports, so this conversion is sound.You can rely on
ng build/ng lintto confirm there are no unexpected diagnostics from these type-only import changes.src/lib/pipes/to-array/to-array.pipe.ts (1)
1-1: PipeTransform as a type-only import is appropriate
PipeTransformis only used in theimplementsclause, so importing it as a type while keepingPipeas a value import is correct and has no runtime impact.src/lib/pipes/to-number/to-number.pipe.ts (1)
1-1: Type-onlyPipeTransformimport here is also correct
ToNumberPipeonly usesPipeTransformin theimplementsclause, so making it a type-only import is consistent and leaves runtime behavior unchanged.package.json (1)
29-61: Angular 21 / TypeScript / tooling versions are compatibleThe devDependencies upgrades are well-aligned: Angular 21 requires TypeScript >=5.9 <6.0, and the pinned version
typescript ~5.9.3satisfies this. Similarly,typescript-eslint ^8.48.0supports TypeScript 5.9.x, andng-packagr ^21.0.0explicitly supports Angular 21 with matching TS requirements. The major version numbers (Angular 21, angular-eslint 21, ng-packagr 21) are intentionally coordinated. No known incompatibilities exist between these specific versions.src/login/pages/update-email/update-email.component.ts (1)
2-7: Type-only imports match pure type usage
TemplateRef,Type, andUserProfileFormFieldsComponentare only used in generic/type positions (viewChild andinput<Type<...>>), so marking them astypeimports is safe and aligns with stricter TS/ESLint settings.src/login/pages/register/register.component.ts (1)
2-20: Lifecycle and component generics correctly converted to type-only imports
OnInit,OnDestroy,Type, andUserProfileFormFieldsComponentare only used as interfaces/generics (implements,input<Type<...>>), so switching them totypeimports is correct and keeps runtime bundle lean.src/login/pages/login-update-profile/login-update-profile.component.ts (1)
2-7: Type-only imports are consistent with usage
TemplateRef,Type, andUserProfileFormFieldsComponentare only referenced in type positions (viewChild andinput<Type<...>>), so marking them astypeimports is appropriate and behavior-preserving.src/login/defaultPage.ts (1)
53-55: Dynamic imports unchanged; error message improved with pageId contextThe dynamic imports remain semantically identical (same module paths and component symbols), and the updated default error now includes the
pageId, which improves debugging without changing control flow.Also applies to: 93-95, 125-127, 129-131, 145-147, 149-151, 153-153
src/login/template/template.component.ts (1)
2-18: EffectRef and KcContext safely switched to type-only imports
EffectRefis only used as a type annotation for#effectRef, andKcContextonly as the generic parameter toinject<KcContext>. Their runtime counterparts aren’t referenced, so converting these imports totypepreserves behavior and matches the type-import pattern elsewhere.Also applies to: 25-25
src/eslint.config.js (1)
2-5: ConfirmdefineConfig(...rootConfig)matches the new root config shapeThis now delegates to
defineConfiginstead oftseslint.config, but still spreadsrootConfig. That’s fine only ifrootConfigis an array of flat config fragments intended to be spread intodefineConfig; if../eslint.config.jswas also updated to return a already-wrapped config, this may double-wrap or change behavior. Please verify ESLint still runs as expected with this file (e.g., run your lint script and check that TS rules and parser settings are applied as before).src/login/directives/kc-class/kc-class.directive.ts (1)
3-15: DoCheck and ClassKey correctly converted to type-only imports
DoCheckis only used viaimplements DoCheckandngDoCheck, andClassKeyappears solely in type aliases and generics. Marking both astypeimports is safe and consistent with type-only import conventions.src/lib/pipes/kc-sanitize/kc-sanitize.pipe.ts (1)
1-9: PipeTransform and Safe symbols used purely as types*
PipeTransformand the Safe* types are only referenced in the pipe’s implements clause and return type annotation; converting them totypeimports is correct and leaves the runtime behavior (viaDomSanitizer) untouched.src/account/services/account-resource-injector/account-resource-injector.service.ts (1)
3-3: LGTM!The type-only import for
Scriptis correct since it's only used as a type annotation in theinsertAdditionalScriptsmethod signature at line 38. This aligns with theverbatimModuleSyntax: truecompiler option and the newconsistent-type-importsESLint rule.src/account/template/template.component.ts (2)
7-7: LGTM!The type-only imports for
EffectRefandTypefrom@angular/coreare correct:
EffectRefis only used as a type annotation for the private field#effectRefTypeis only used as a generic type parameter ininput<Type<unknown>>()Also applies to: 12-12
26-27: LGTM!The type-only imports for
ClassKeyandObservableare appropriate since both are only used in type positions (field type annotations and generic parameters).src/account/DefaultPage/DefaultPage.ts (1)
1-3: LGTM!The type-only imports for
TypeandClassKeyare correct since both are only used in type annotation positions (return type and parameter type).src/account/directives/kc-class/kc-class.directive.ts (1)
5-5: LGTM!The type-only imports are correct:
DoCheckis an interface used in theimplementsclause, which is erased at compile timeClassKeyis only used in type positionsBoth align with
verbatimModuleSyntax: true.Also applies to: 13-13
eslint.config.js (2)
7-10: LGTM!The migration to
defineConfigfromeslint/configis the correct approach for ESLint's flat config format. This provides better type checking and IDE support for the configuration.
33-40: Good addition enforcing type-only imports.The
consistent-type-importsrule withinline-type-importsfixStyle explains the pattern seen across the codebase changes (e.g.,import { type Script }rather thanimport type { Script }). This ensures consistent enforcement going forward.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
stories/login/pages/login-otp.stories.ts (1)
22-28: Consider giving each OTP credential a uniqueidAll but the first credential share
id: 'credential2', which is atypical for Keycloak credential IDs and may make this story less representative of real data.You could, for example, adjust the IDs like:
- { id: 'credential2', userLabel: 'Device 3' }, - { id: 'credential2', userLabel: 'Device 4' }, - { id: 'credential2', userLabel: 'Device 5' }, - { id: 'credential2', userLabel: 'Device 6' } + { id: 'credential3', userLabel: 'Device 3' }, + { id: 'credential4', userLabel: 'Device 4' }, + { id: 'credential5', userLabel: 'Device 5' }, + { id: 'credential6', userLabel: 'Device 6' }
📜 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 (53)
package.json(3 hunks)stories/account/KcContextMock.ts(1 hunks)stories/account/KcPage.ts(1 hunks)stories/account/KcPageStory.ts(2 hunks)stories/account/i18n.ts(1 hunks)stories/account/pages/KcPageStory.ts(0 hunks)stories/account/pages/account.stories.ts(1 hunks)stories/account/pages/applications.stories.ts(1 hunks)stories/account/pages/federated-identity.stories.ts(1 hunks)stories/account/pages/log.stories.ts(1 hunks)stories/account/pages/password.stories.ts(1 hunks)stories/account/pages/sessions.stories.ts(1 hunks)stories/account/pages/totp.stories.ts(1 hunks)stories/login/KcContextMock.ts(1 hunks)stories/login/KcPage.ts(1 hunks)stories/login/KcPageStory.ts(2 hunks)stories/login/i18n.ts(1 hunks)stories/login/pages/code.stories.ts(1 hunks)stories/login/pages/delete-account-confirm.stories.ts(1 hunks)stories/login/pages/delete-credential.stories.ts(1 hunks)stories/login/pages/error.stories.ts(1 hunks)stories/login/pages/frontchannel-logout.stories.ts(1 hunks)stories/login/pages/idp-review-user-profile.stories.ts(1 hunks)stories/login/pages/info.stories.ts(1 hunks)stories/login/pages/login-config-totp.stories.ts(1 hunks)stories/login/pages/login-idp-link-confirm-override.stories.ts(1 hunks)stories/login/pages/login-idp-link-confirm.stories.ts(1 hunks)stories/login/pages/login-idp-link-email.stories.ts(1 hunks)stories/login/pages/login-oauth-grant.stories.ts(1 hunks)stories/login/pages/login-oauth2-device-verify-user-code.stories.ts(1 hunks)stories/login/pages/login-otp.stories.ts(1 hunks)stories/login/pages/login-page-expired.stories.ts(1 hunks)stories/login/pages/login-passkeys-conditional-authenticate.stories.ts(1 hunks)stories/login/pages/login-password.stories.ts(1 hunks)stories/login/pages/login-recovery-authn-code-config.stories.ts(1 hunks)stories/login/pages/login-recovery-authn-code-input.stories.ts(1 hunks)stories/login/pages/login-reset-otp.stories.ts(1 hunks)stories/login/pages/login-reset-password.stories.ts(1 hunks)stories/login/pages/login-update-password.stories.ts(1 hunks)stories/login/pages/login-update-profile.stories.ts(1 hunks)stories/login/pages/login-username.stories.ts(1 hunks)stories/login/pages/login-verify-email.stories.ts(1 hunks)stories/login/pages/login-x509-info.stories.ts(1 hunks)stories/login/pages/login.stories.ts(1 hunks)stories/login/pages/logout-confirm.stories.ts(1 hunks)stories/login/pages/register.stories.ts(1 hunks)stories/login/pages/saml-post-form.stories.ts(1 hunks)stories/login/pages/select-authenticator.stories.ts(1 hunks)stories/login/pages/terms.stories.ts(1 hunks)stories/login/pages/update-email.stories.ts(1 hunks)stories/login/pages/webauthn-authenticate.stories.ts(1 hunks)stories/login/pages/webauthn-error.stories.ts(1 hunks)stories/login/pages/webauthn-register.stories.ts(1 hunks)
💤 Files with no reviewable changes (1)
- stories/account/pages/KcPageStory.ts
✅ Files skipped from review due to trivial changes (11)
- stories/account/pages/log.stories.ts
- stories/login/KcContextMock.ts
- stories/account/KcContextMock.ts
- stories/account/pages/account.stories.ts
- stories/login/pages/frontchannel-logout.stories.ts
- stories/login/pages/login-update-password.stories.ts
- stories/login/pages/login-x509-info.stories.ts
- stories/login/pages/login.stories.ts
- stories/login/pages/delete-account-confirm.stories.ts
- stories/login/pages/select-authenticator.stories.ts
- stories/account/pages/federated-identity.stories.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧬 Code graph analysis (2)
stories/account/KcPageStory.ts (2)
stories/account/KcPage.ts (1)
getKcPage(8-18)stories/login/KcPage.ts (1)
getKcPage(10-22)
stories/login/KcPageStory.ts (1)
stories/login/KcPage.ts (1)
getKcPage(10-22)
⏰ 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: test
🔇 Additional comments (42)
stories/login/pages/login-oauth2-device-verify-user-code.stories.ts (1)
1-1: LGTM! Correct use of type-only import.The conversion to
import typeis appropriate sinceMetaandStoryObjare only used in type positions (lines 4 and 13), not at runtime. This change aligns with TypeScript best practices and supports theverbatimModuleSyntaxcompiler option.stories/login/pages/login-recovery-authn-code-input.stories.ts (1)
1-1: LGTM! Correct use of type-only import.The conversion to
import typeis appropriate sinceMetaandStoryObjare used exclusively as types in this file (lines 4 and 14). This reduces the runtime bundle size and aligns with TypeScript best practices, especially whenverbatimModuleSyntaxis enabled.stories/login/pages/login-passkeys-conditional-authenticate.stories.ts (1)
1-1: LGTM! Type-only import correctly applied.The conversion to
import typeis appropriate since bothMetaandStoryObjare exclusively used as type annotations. This aligns with the Angular 21 upgrade andverbatimModuleSyntaxconfiguration changes.stories/login/pages/login-reset-password.stories.ts (1)
1-1: LGTM! Correct use of type-only import.The conversion to
import typeis appropriate sinceMetaandStoryObjare used exclusively in type positions. This aligns with TypeScript best practices and theverbatimModuleSyntaxcompiler option mentioned in the PR changes.stories/login/pages/login-oauth-grant.stories.ts (1)
1-1: Type‑only import for Storybook types is appropriate here
MetaandStoryObjare only used as type parameters in this file, so converting to aimport typekeeps them out of the runtime bundle and aligns with stricter TS module settings. No further changes needed in this file.Please just confirm that your TS config (especially
module/verbatimModuleSyntax) is aligned across the repo soimport typeworks consistently in all story files.stories/login/pages/login-recovery-authn-code-config.stories.ts (1)
1-4: Type‑only Storybook imports look correctUsing
import typeforMetaandStoryObjis appropriate here since they’re only used in type positions (Meta<KcPageStory>,StoryObj<KcPageStory>). This should play nicely with stricter TS/Angular 21 module settings and avoids unnecessary runtime imports.Please just confirm the TS/Storybook build passes so we know there are no other files relying on
Meta/StoryObjas runtime values.stories/login/pages/login-config-totp.stories.ts (1)
1-1: LGTM! Correct use of type-only import.The conversion to a type-only import is appropriate since
MetaandStoryObjare used exclusively as types. This reduces bundle size and aligns with theverbatimModuleSyntaxcompiler option mentioned in the PR.stories/login/pages/webauthn-register.stories.ts (1)
1-1: LGTM! Correct use of type-only import.The type-only import is correct as both symbols are used exclusively for type annotations. This change is consistent with the PR-wide migration pattern.
stories/login/pages/idp-review-user-profile.stories.ts (1)
1-1: LGTM! Correct use of type-only import.The type-only import correctly reflects that
MetaandStoryObjare used exclusively as types, improving type safety and bundle size.stories/login/pages/code.stories.ts (1)
1-1: LGTM! Correct use of type-only import.The type-only import is appropriate and aligns with the Angular 21 upgrade initiative to use modern TypeScript patterns.
stories/login/pages/login-username.stories.ts (1)
1-1: LGTM! Correct use of type-only import.The change correctly uses type-only imports for symbols that are never used as runtime values, improving code quality.
stories/login/pages/login-update-profile.stories.ts (1)
1-1: LGTM! Correct use of type-only import.The type-only import is correct and consistent with the broader refactoring pattern across the PR.
stories/login/pages/webauthn-authenticate.stories.ts (1)
1-1: LGTM! Correct use of type-only import.The type-only import is appropriate for these Storybook types, ensuring they don't appear in the final JavaScript bundle.
stories/login/pages/webauthn-error.stories.ts (1)
1-1: LGTM! Correct use of type-only import.The type-only import correctly ensures that
MetaandStoryObjare used exclusively as types, completing the consistent migration pattern across all story files.stories/login/pages/info.stories.ts (1)
1-1: LGTM! Type-only import is appropriate.The conversion to
import typeis correct sinceMetaandStoryObjare only used as type annotations. This aligns with the PR's migration to type-only imports and works well withverbatimModuleSyntax.stories/login/pages/delete-credential.stories.ts (1)
1-1: LGTM! Type-only import is appropriate.stories/login/pages/login-reset-otp.stories.ts (1)
1-1: LGTM! Type-only import is appropriate.stories/login/pages/logout-confirm.stories.ts (1)
1-1: LGTM! Type-only import is appropriate.stories/login/pages/login-idp-link-confirm.stories.ts (1)
1-1: LGTM! Type-only import is appropriate.stories/login/pages/login-idp-link-email.stories.ts (1)
1-1: LGTM! Type-only import is appropriate.stories/login/pages/login-page-expired.stories.ts (1)
1-1: LGTM! Type-only import is appropriate.stories/login/pages/register.stories.ts (1)
1-2: LGTM! Type-only imports are appropriate.Both
Meta/StoryObjandAttributeare correctly converted to type-only imports since they're only used for type annotations (satisfieson lines 67 and 103).stories/login/pages/login-otp.stories.ts (1)
1-1: Type‑only Storybook import is appropriateMeta and StoryObj are only used as types, so converting to
import typeis correct and keeps them out of the runtime bundle. Just ensure your TypeScript/Storybook build stays green after the Angular 21 upgrade.stories/account/i18n.ts (1)
1-1:ThemeNameas a type‑only import looks correct
ThemeNameis only used in the generic forwithThemeName, so switching toimport typeis appropriate and avoids an unnecessary runtime import.stories/login/pages/login-verify-email.stories.ts (1)
1-1: Meta/StoryObj type‑only import matches Storybook usageMeta and StoryObj are used purely for typing, so
import typeis the right choice and should not alter story behavior.stories/login/pages/update-email.stories.ts (1)
1-1: Consistent use of type‑only Meta/StoryObj importThe move to
import typealigns this story with the rest of the Storybook files and correctly reflects that Meta/StoryObj are compile‑time only.stories/account/pages/sessions.stories.ts (1)
1-1: Type‑only Meta/StoryObj import is appropriate hereMeta and StoryObj are only used for typing the
metaandStoryalias, so converting them to a type‑only import is correct and avoids an unnecessary runtime dependency.stories/login/pages/login-idp-link-confirm-override.stories.ts (1)
1-1: Storybook typings correctly moved to a type‑only importUsing
import typefor Meta and StoryObj matches their usage in this file and keeps the runtime surface minimal.stories/login/i18n.ts (1)
1-1:ThemeNameimported as a type fits the i18n builder usage
ThemeNameis only used generically inwithThemeName<ThemeName>(), so making it a type‑only import is appropriate and keeps runtime imports lean.stories/account/KcPage.ts (1)
1-4: Type‑only imports for ClassKey, KcPage, and KcContext are well‑applied
ClassKeyandKcContextare only used for typing, andKcPageis only referenced in the function return type, so moving them to type‑only imports is appropriate and keeps the runtime surface limited togetDefaultPageComponentandTemplateComponent.stories/account/pages/password.stories.ts (1)
1-1: LGTM!The type-only import is appropriate here since
MetaandStoryObjare only used as type annotations, not as runtime values. This aligns with theverbatimModuleSyntaxcompiler option being added in this PR.stories/account/pages/applications.stories.ts (1)
1-1: LGTM!Correct use of type-only import for
MetaandStoryObjwhich are only used as type annotations.stories/login/pages/login-password.stories.ts (1)
1-1: LGTM!Type-only import is correctly applied for symbols used only in type positions.
stories/login/pages/error.stories.ts (1)
1-1: LGTM!Type-only import correctly reflects that
MetaandStoryObjare used purely for type annotations.stories/account/pages/totp.stories.ts (1)
1-1: LGTM!Type-only import is appropriate for these Storybook types that are only used for type annotations.
stories/login/pages/saml-post-form.stories.ts (1)
1-1: LGTM! Correct type-only import.Converting
MetaandStoryObjto type-only imports reduces the runtime bundle size and follows TypeScript best practices, as these are only used for type annotations.stories/account/KcPageStory.ts (2)
1-7: LGTM! Improved import declarations.The expanded imports correctly use type-only imports for
OnInitandType, which are only used as type annotations. This reduces runtime dependencies and aligns with the TypeScript best practices in Angular 21.
43-47: Good practice: ensuring change detection after async operations.Injecting
ChangeDetectorRefand callingmarkForCheck()after the asyncgetKcPage()resolves ensures the view updates correctly. This is defensive coding that works with both default and OnPush change detection strategies.stories/login/pages/terms.stories.ts (1)
1-1: LGTM! Consistent type-only import pattern.This change mirrors the pattern applied across other story files in the PR, correctly converting
MetaandStoryObjto type-only imports.stories/login/KcPageStory.ts (2)
1-11: LGTM! Proper import declarations and type-only usage.The import changes correctly introduce
ChangeDetectorRefand convertOnInit,Type, andStoryContextto type-only imports. This matches the pattern established instories/account/KcPageStory.tsand maintains consistency across the codebase.
45-51: Correct change detection handling after async page load.The addition of
ChangeDetectorRefinjection and themarkForCheck()call ensures the component view updates after the asynchronous page component resolution. This mirrors the implementation in the account story component and follows Angular best practices for async operations.stories/login/KcPage.ts (1)
1-6: LGTM! Precise type-only import usage.The import changes correctly distinguish between type-only imports (
ClassKey,KcPage,KcContext) and runtime imports (TemplateComponent). This is accurate because:
ClassKey,KcPage, andKcContextare only used in type positionsTemplateComponentis returned as a value (line 15) and requires a runtime importThese changes optimize the bundle and align with the broader type-import refinements across the PR.
|
Fantastic, thank you very much @luca-peruzzo! (About everything else, I'm deep deep into a security rabithole but I'll claim back eventually) |
Thanks to @lekhmanrus for his contribution on this.
Summary by CodeRabbit
Bug Fixes
Chores
Refactor
Removed
✏️ Tip: You can customize this high-level summary in your review settings.