firebase / firebase-js-sdk

Firebase Javascript SDK
https://firebase.google.com/docs/web/setup
Other
4.78k stars 876 forks source link

Don't wait resolving firebase.auth() when enablePersistence() is used #3302

Open mu3 opened 4 years ago

mu3 commented 4 years ago

Describe the problem

Once signed-in, internally Firestore starts to wait for auth to give its first credential changed event before enabling persistence. The behaviour remains the same even if firebase.auth() is completely removed. This verification doesn't actually change anything even if the Firestore rules have changed to return 403 on requested resources: cache is returned as-is, every time.

Steps to reproduce / Relevant Code:

firebase.firestore().enablePersistence();
console.log('Fetching data...');
firebase.firestore().doc('foo/bar').onSnapshot(snap => {
  console.log('Data source:', snap.metadata.fromCache ? 'cache' : 'server')
  console.log(snap.data())
})
// firebase.auth().signInAnonymously(); // Single call is enough
Fetching data...
‼‼️‼‼️ Waiting for internal call to firebase.auth()...
Data source: cache
► {hotdogStatus: "Not a sandwich"}
Uncaught Error in onSnapshot: FirebaseError: Missing or insufficient permissions.

firebase.auth().signOut() makes Firestore act offline-first again. The same as removing IndexedDB > firebaseLocalStorageDb > firebaseLocalStorage > 0

Fetching data...
Data source: cache
► {hotdogStatus: "Not a sandwich"}
Uncaught Error in onSnapshot: FirebaseError: Missing or insufficient permissions.

Expected behavior:

When enablePersistence() is used along auth(), return cache right away. It doesn't seem like a security breach. Especially because cache is returned in any case, just with a delay. I don't feel comfortable using Local Storage workarounds to duplicate data:

import useLocalStorage from '@rehooks/local-storage';
...
const [data, setData] = useLocalStorage('cachedData', {});
...
firebase.firestore().enablePersistence()
firebase.firestore().doc('foo/bar').onSnapshot(snap => setData(snap.data()) )
firebase.auth();

Describe your environment

wu-hui commented 4 years ago

Hi @mu3

The local indexeddb instance is associated with signed in user (or no sign-in), each user will use a different instance to ensure data separation. That is why we need this behavior when persistence is enabled.

From your description, it seems like you are seeing data leaking to other users through local storage? Or is it happen to be data shared across users?

Thanks.

mu3 commented 4 years ago

I mean… I expect web app to work like any other native app. When I open up Tweetbot my whole feed is shown immediately since it was cached under my account. It would be unexpected if the user changed while the app was closed. IMO the cache should change in response to the credentials change. Why to verify and wait for something that is not expected to change by itself? I think cache should be returned first (as the data was received under authorized user), and then credentials could be verified and cache handled appropriately. I believe today web apps are mostly used on personal devices like any other apps and not in public libraries. And in addition to that, the web SDK doesn’t enable offline cache by default in order for developer to include “remember me” checkbox during the authorization if needed.

kevinrenskers commented 3 years ago

100% agree with all this. In my web app, where I use Auth and Firestore and have enabled persistence, there is always a delay until the UI can show me my logged in status, avatar, etc. I would really imagine that this info would be available immediately, from cache, which is then updated behind the scenes as needed.

The UI now feels too slow when opening the website again after having been logged in before, or when pressing refresh for some reason. All your stuff should be there instantly, but instead it's popping in a split second later, because it's not using the cache. Please change this.

gustavopch commented 2 years ago

Related comment I made in another issue asking for onAuthStateChanged() to resolve immediately with the cached auth state: https://github.com/firebase/firebase-js-sdk/issues/4133#issuecomment-1159207679

chdsbd commented 10 months ago

I've noticed this delay too. It gives firestore a really poor user experience when loading pages. Other libraries could be used to implement another layer of caching, but firestore already has a cache, so why can't we just use it immediately?