Skip to content

build: require firebase ^12.18.0 and wrap the functions it adds - #3761

Merged
armando-navarro merged 3 commits into
angular:mainfrom
armando-navarro:a38-pr-b-firebase-floor
Sep 2, 2026
Merged

build: require firebase ^12.18.0 and wrap the functions it adds#3761
armando-navarro merged 3 commits into
angular:mainfrom
armando-navarro:a38-pr-b-firebase-floor

Conversation

@armando-navarro

Copy link
Copy Markdown
Collaborator

Checklist

Description

tools/build.ts writes src/<module>/firebase.ts from the type declarations of whatever firebase is installed, so which functions @angular/fire wraps is decided by the version the build resolves rather than by a list anyone maintains. This raises that version, and regenerates. #3756 has the background.

Three things follow from the way the generator works:

  • The version the build resolves has been 12.4.0 while npm latest reached 12.18.0.
  • Everything Firebase added in between still reaches callers through the export * line, but arrives with none of the wrapper's behavior and nothing saying it is different.
  • The range the repository builds against, the range the published package declares and the schematics constant all have to move together, or alignFirebaseVersion starts writing a range the library does not install against.

What regenerating changes

  • src/messaging/firebase.ts is the only generated file that moves. It gains onRegistered, onUnregistered, register and unregister.
  • Nothing else changes, because build: classify the firebase symbols a version raise would newly wrap #3759 already classified every other symbol the raise makes visible.
  • Those entries keep their symbols out of the generated files while the export * line continues to carry them to callers.

What comes out

What this means for someone upgrading

  • @angular/fire declares firebase as a regular dependency rather than a peer, so a workspace holding an older firebase that upgrades @angular/fire without running a schematic now resolves two copies instead of one, and the two reject each other's objects at runtime. Measured under Testing rather than assumed.
  • alignFirebaseVersion rewrites the declared range for anyone who runs ng add or ng update, and it picks up the new range with no further change.
  • ng add should warn when the workspace has more than one version of firebase installed #3754 covers telling the people who run neither.

Specs

  • Two alignFirebaseVersion specs used ^12.6.0 to stand for a range a workspace declares that is already inside what the library requires.
  • ^12.6.0 stops being inside that range at 12.18.0, so both specs would have started asserting the opposite of what they say.
  • They now derive that value from firebaseVersionRange, so the next raise cannot leave them wrong.

Docs

  • docs/messaging.md wrote the firebase version into two gstatic service worker URLs, which is only correct on the day it is written. It now uses a placeholder, matching docs/compat/messaging/messaging.md.
  • docs/version-21-upgrade.md names the new range, and no longer implies that only a firebase 11 workspace ends up with two copies.

Testing

npm run build && npm run build:jasmine && npm run test:node
  • 227 specs, 0 failures.
  • npx ng lint passes.
  • The karma suite passes against the emulators: 107 executed, 48 skipped, 0 failures.

Two results are worth stating separately, because neither is visible in the diff.

The override entries are doing the work, not the version

The duplicate copy was measured, not assumed

A fresh ng new app, firebase set to 12.10.0, this build of @angular/fire installed, no schematic run:

├─┬ @angular/fire@21.0.0-rc.0
│ ├── firebase@12.18.0
│ └─┬ rxfire@6.2.0
│   └── firebase@12.10.0 deduped
└── firebase@12.10.0

The split runs through the library's own dependencies: @angular/fire gets 12.18.0 while rxfire, which @angular/fire depends on, gets the app's 12.10.0. The same app with no firebase of its own resolves a single 12.18.0 for everything.

One judgment call I would particularly like a second opinion on: register is wrapped with blockUntilFirst at its default, which means a call made before notification permission is granted holds the application unstable while the browser prompt is open. getToken has done the same for years, and setting false would drop zone and injection-context re-entry from the returned promise rather than just the pending task, so I left it alone. If you would rather that not ship, say so and I will take it out of this PR.

Fixes #3756

@armando-navarro armando-navarro added bump: patch comp: build/pipeline Build, bundling, packaging, release pipeline. comp: docs Documentation. comp: messaging Cloud Messaging / FCM (src/messaging). comp: schematics ng add / deploy schematics (src/schematics). comp: zones Change detection / zone.js / zoneless. dependencies Pull requests that update a dependency file type: bug Defect: expected behavior doesn't happen. labels Sep 1, 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.

Approving. On your register question: leave it. The reason is stronger than "getToken has done the same for years": register and getToken have identical permission preambles in @firebase/messaging 12.18.0, both await Notification.requestPermission() when permission is default, then throw permission-blocked. It isn't a new hazard, it's the same one, now on the API that replaces the one already carrying it. Your reasoning about the alternative holds too: at blockUntilFirst: false a promise falls to zones.ts:153-161 and comes back bare, losing the runInInjectionContext re-entry that 169-180 gives it. There's no "re-enter but don't block" setting.

Ran the rest. npm ci at 12.18.0 then npm run generate reproduces the committed files byte for byte, so messaging really is the only one that moves. 227 specs, 0 failures. Forcing alreadyCompatible = false turns both rewritten specs red, so they're live; simulating the next raise to ^12.20.0 leaves them passing and trips only the pre-existing src/package.json drift guard, which is the right thing to fire.

One follow-up, not for this PR: 12.18.0 deprecates both getToken and deleteToken in favour of register/onRegistered/onUnregistered, and docs/messaging.md:81 still imports both. Worth an issue.

Also, this branch carries #3759's three commits, so it'll want a rebase once that squash-merges.

The generated src/<module>/firebase.ts files are written from the type
declarations of whatever firebase is installed, so the set of functions
AngularFire wraps is decided by the version the build resolves. That was
12.4.0 while npm latest reached 12.18.0, leaving newly added functions
reaching callers through the star export with no zone integration and no
pending-task registration.

Raising the required version wraps them. Everything the override entries
in angular#3759 classified as unwrapped stays out.

getImagenModel loses its entry because firebase removed the symbol in
12.18.0, so the generator no longer sees it. The exemption list it sat in
is empty now and stays as the place the next unclassified name goes.

Fixes angular#3756
firebase 12.18 deprecates getToken and deleteToken and says both will be
removed, pointing callers at register with onRegistered, and unregister
with onUnregistered.

The replacement is a different model rather than a rename, so the page now
explains it before the example.

The Node send example keeps its token field, which Firebase says still
accepts an installation ID during the migration. It does not switch to the
fid field Firebase recommends, because firebase-admin 13.5.0 does not
declare one.
The comment claimed a name allowed for one entry point is not allowed for
the rest. The check does not work that way. exportsSeenPerOverrides is
keyed by the overrides object, and firestore and firestore/lite are handed
the same firestoreOverrides, so a name listed for one silences the check
for the other.

The list is empty today, so nothing behaves differently. The comment is
what a future reader would have relied on when adding the next name.
@armando-navarro

Copy link
Copy Markdown
Collaborator Author

On the docs point, I have updated the docs here rather than filing it, since the PR template asks whether docs are included and this is the change that makes the replacement calls available. Pushed as a follow-up commit.

Two things I found while writing it that are worth knowing:

  • onRegistered has to be listening before register runs. register throws No onRegistered callback handler was provided or registered otherwise, so the old example could not simply have its imports swapped. The new section says so before the sample.
  • Firebase's Admin SDK send guide says the token field still accepts an installation ID during the migration, so the guide's Node example keeps working unchanged. It recommends a dedicated fid field, which I deliberately did not put in the sample because firebase-admin 13.5.0 does not declare one.

@armando-navarro
armando-navarro merged commit 97f61a4 into angular:main Sep 2, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bump: patch comp: build/pipeline Build, bundling, packaging, release pipeline. comp: docs Documentation. comp: messaging Cloud Messaging / FCM (src/messaging). comp: schematics ng add / deploy schematics (src/schematics). comp: zones Change detection / zone.js / zoneless. dependencies Pull requests that update a dependency file type: bug Defect: expected behavior doesn't happen.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Raise the firebase version AngularFire is built against, so functions added since 12.4.0 are wrapped

2 participants