Skip to content

style: use the optional chains typescript-eslint 8.71 asks for - #3797

Draft
armando-navarro wants to merge 1 commit into
angular:mainfrom
armando-navarro:a69-optional-chain-lint
Draft

armando-navarro wants to merge 1 commit into
angular:mainfrom
armando-navarro:a69-optional-chain-lint

Conversation

@armando-navarro

Copy link
Copy Markdown
Collaborator

Caution

Do not merge until the 21.0.x branch is cut

This is part of the Angular 22 upgrade and ships in 22.0.0-rc.0. Merge it after 21.0.x is cut from main.

Refs #3696

This rewrites two checks as optional chains, so lint keeps passing once the Angular 22 upgrade brings in a newer @typescript-eslint.

Changes

  • src/app/app.module.ts: provided && provided.length === 1 becomes provided?.length === 1.
  • src/compat/firebase.app.module.ts: app && app.name === config.name becomes app?.name === config.name.

Both rewrites keep behavior. An undefined provided still falls through to getApp(), and since config.name is set to a non-empty string two lines above the check, a missing app still never matches it.

Why

  • Angular 22 requires TypeScript 6.
  • @typescript-eslint 8.46, the version in today's lockfile, accepts only TypeScript below 6.0, so the Angular 22 upgrade moves it to 8.71.
  • 8.71 reports @typescript-eslint/prefer-optional-chain on these two lines, so npm run lint fails.

Verification

With @typescript-eslint 8.71.1 on the Angular 22 upgrade, eslint reports exactly these two errors on the old lines and none on the new ones.

There are no new tests, since the existing specs already run both lines through provideFirebaseApp and AngularFireModule.initializeApp.

The Angular 22 upgrade needs @typescript-eslint 8.71 for TypeScript 6,
and that release reports prefer-optional-chain on these two checks, so
lint fails once it lands. Both rewrites keep behavior. An undefined
provided list still falls through to getApp(). config.name is always a
non-empty string, so a missing app still never matches it.
@armando-navarro armando-navarro added comp: compat Compatibility layer for the pre-modular API (src/compat). comp: core FirebaseApp DI core (src/app). type: chore Maintenance with no user-facing behavior change. labels Oct 8, 2026
@armando-navarro armando-navarro added this to the 22.0.0-rc.0 milestone Oct 8, 2026

@tyler-reitz tyler-reitz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both rewrites check out. Holding the approval until 21.0.x is cut, since approval is the only gate here and would make this mergeable early. Ping me once the branch exists.

One correction to the reasoning, which does not change the fix. prefer-optional-chain is already enabled as error in 8.46's stylistic-type-checked, which eslint.config.js:28 pulls in, so the rule has been live all along. I linted both forms under both versions: 8.46.0 reports nothing on either, 8.71.1 reports exactly these two and nothing on the new lines. So the rule's detection broadened between the two versions rather than the rule arriving with 8.71.

On behavior, I ran the old and new forms side by side: five inputs for provided, and fourteen find combinations crossing both config names with arrays holding null, undefined, matching and non-matching apps. No divergences. Worth recording why the second one is safe: the two forms do differ when config.name is nullish, where the old form returns the matching object and the new one returns the null element, which sends existingApp || initializeApp(...) down different branches. It cannot happen because name falls back to '[DEFAULT]' and config.name = config.name || name two lines up, exactly as you say. That assignment is now load-bearing for the rewrite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: compat Compatibility layer for the pre-modular API (src/compat). comp: core FirebaseApp DI core (src/app). type: chore Maintenance with no user-facing behavior change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants