Repository navigation
style: use the optional chains typescript-eslint 8.71 asks for - #3797
armando-navarro wants to merge 1 commit into
Conversation
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.
tyler-reitz
left a comment
There was a problem hiding this comment.
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.
Caution
Do not merge until the
21.0.xbranch is cutThis is part of the Angular 22 upgrade and ships in 22.0.0-rc.0. Merge it after
21.0.xis cut frommain.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 === 1becomesprovided?.length === 1.src/compat/firebase.app.module.ts:app && app.name === config.namebecomesapp?.name === config.name.Both rewrites keep behavior. An undefined
providedstill falls through togetApp(), and sinceconfig.nameis set to a non-empty string two lines above the check, a missingappstill never matches it.Why
@typescript-eslint8.46, the version in today's lockfile, accepts only TypeScript below 6.0, so the Angular 22 upgrade moves it to 8.71.@typescript-eslint/prefer-optional-chainon these two lines, sonpm run lintfails.Verification
With
@typescript-eslint8.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
provideFirebaseAppandAngularFireModule.initializeApp.