Repository navigation
Migrate the frontend to the modular Firebase SDK - #1714
Open
HugoGresse wants to merge 3 commits into
Open
HugoGresse wants to merge 3 commits into
HugoGresse wants to merge 3 commits into
Conversation
Replace the namespaced/compat API (firebase/compat/*, db.collection().doc(), firebase.firestore.FieldValue, compat auth/storage/functions/analytics) with the modular v9+ functions in the feedback, admin and superadmin apps. Paths, queries, listeners, merge options, error handling and the inviteListener fromCache check are unchanged. - src/firebase.ts: initializeFirestore (long polling on the emulators) + connect*Emulator; initializeAuth with getAuth's persistence and popup resolver, plus debugErrorMap to keep compat's full error messages. - Storage moves to an admin-only module, out of the voters' entry chunk. - firebaseui@6 still needs a compat Auth: compat stays only in StyledFirebaseAuth, which wraps the modular [DEFAULT] app and Auth, and is now lazy-loaded with the sign-in form. - Specs mock the modular functions and assert the document paths. - Remove addVoteToDb and useFirestoreDocument, which had no callers. Run the Firestore rules spec on @firebase/rules-unit-testing 3.0.4 (last line with a `firebase ^10` peer) instead of the deprecated @firebase/testing. Its firebase@7 tree hoisted @firebase/util@0.3.1, so npm nested private copies of @firebase/auth, util, ... under every firebase@10 package; with firebase/auth and firebase/compat/auth both loaded, the copies failed each other's `instanceof FirebaseError` and firebaseui reported every sign-in error as auth/network-request-failed. The re-lock hoists one copy of each, and a spec guards it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # src/admin/users/core/actions/usersActions.ts
|
Preview for 7a6832f (sandbox project open-feedback-hugo, expires in 7 days): https://open-feedback-hugo--pr-1714-gt4mtqts.web.app |
open-feedback
 |
||||||||||||||||||||||||||||
| Project |
open-feedback
|
| Branch Review |
firestore-modular-sdk
|
| Run status |
|
| Run duration | 02m 15s |
| Commit |
|
| Committer | Hugo Gresse |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
 |
0
|
 |
1
|
 |
0
|
 |
0
|
 |
16
|
| View all changes introduced in this branch ↗︎ | |
…tore-modular-sdk # Conflicts: # config/firestore.rules.spec.js
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Moves the feedback, admin and superadmin apps from the Firebase namespaced/compat API to the modular v9+ SDK (
firebase/firestore,firebase/auth,firebase/storage,firebase/functions,firebase/analytics,firebase/performance), so bundles can be tree-shaken. Behaviour is meant to be identical: same document paths, queries, listeners/unsubscribes,mergeoptions, error dispatches, and thefromCacheoffline check ininviteListener.src/firebase.ts:initializeFirestore(forced long polling on the emulators) +connect*Emulator;initializeAuthwithgetAuth's persistence and popup resolver, plusdebugErrorMapso auth errors keep the full messages compat installed.src/admin/project/utils/storage/firebaseStorage.ts, so the voters' entry chunk no longer ships it.addVoteToDbanduseFirestoreDocumenthad no callers, so they are deleted rather than migrated.What still uses compat, and why
firebase/compat/app+firebase/compat/authremain only insrc/baseComponents/StyledFirebaseAuth.tsx:firebaseui@6.1.0(latest, unmaintained since 2023) needs a compat Auth instance and has a peer offirebase ^9 || ^10.@firebase-oss/ui-react@7needs React 19 and firebase 11/12.The bridge wraps the modular
[DEFAULT]app and reuses the modular Auth instance, including its emulator connection. It is lazy-loaded with the sign-in form, so only the login screens download compat + firebaseui. This also keepsfirebaseon 10.14.1: npm latest is 13.1.0, but firebaseui blocks the upgrade.Dependency change:
@firebase/testing→@firebase/rules-unit-testing@3.0.4The code review found a real regression that this fixes:
@firebase/testingpulled in firebase@7 and hoisted@firebase/util@0.3.1. npm then nested a private copy of@firebase/auth/utilunder every firebase@10 package.firebase/auth(modular) andfirebase/compat/auth(firebaseui) loaded together, the copies failed each other'sinstanceof FirebaseError. firebaseui reported every sign-in error asauth/network-request-failed: a wrong password showed "network error", and account linking and the anonymous-upgrade merge conflict never ran.@firebase/rules-unit-testing3.0.4, the last line with afirebase ^10peer. Its helpers are unchanged:createApp()is an authenticated context, and organizations are seeded with the rules disabled. The re-lock hoists one copy of each@firebase/*package.src/firebaseAuthSingleCopy.spec.tsfails if a future install nests a second@firebase/auth,util,apporcomponent.tslibconsolidation.Bundle (production build, gzip)
index-*.js)StyledFirebaseAuthchunkNot split into stacked PRs
The
src/firebase.tsexports change for every area at once:serverTimestamp,authand the other compat helpers are removed, andfireStoreMainInstanceis now a modularFirestore. Per-area PRs would therefore not build on their own. The change is mechanical per file, so reviewing by folder works:src/feedback,src/core,src/admin/project,src/admin/{auth,users,organization},src/superAdmin.Test plan
npm run typecheck,npm run lint(same 4 pre-existingnoConsolewarnings asmain),npm run lint:i18nnpm test(emulators:exec firestore): 48 files, 476 tests, including the migrated rules spec?inviteId=/code-review max: the findings that hold up are fixed, and the remaining ones are listed belowNo screenshots: nothing rendered changes, apart from firebaseui error messages that are correct again.
Follow-ups (not in this PR)
signInFailuremerge handler callscurrentUser.delete()aftersignInWithCredential, which signs the new user out;deleteImageIfPossibleURL-decodes paths incorrectly, so old images with spaces in their name are never deleted.getCountFromServerwould cost about 1 read per 1,000 votes.🤖 Generated with Claude Code