firebase / firebase-js-sdk

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

getReactNativePersistence breaks React Native Fast Refresh #6231

Closed Nantris closed 1 year ago

Nantris commented 2 years ago

Describe your environment

Describe the problem

Firebase@9.x Auth requires getReactNativePersistence on React Native - but including the relevant code for getReactNativePersistence causes the auth listener logic in our app to fire every time a change is made, crashing our app (without apparent error message) and preventing us from using Fast Refresh at all.

Steps to reproduce:

Unsure of full steps that may be required. For us, including the code below triggers it in logic that otherwise worked in Firebase@8.x and in Firebase@9.x for a while.

Relevant Code:

  initializeAuth(firebaseApp, {
    persistence: getReactNativePersistence(AsyncStorage),
  });
Nantris commented 2 years ago

It may be that this only occurs during debugging but I can't confirm that at the moment now due to a major overhaul in our tooling.

ishmaelmoreno commented 2 years ago

@Slapbox does this still happen for you?

Nantris commented 2 years ago

I think no, but I'm not sure of that.

I don't think the possible resolution is related to any change in Firebase though, but rather that we abandoned react-native-debugger in favor of Flipper.

Nantris commented 2 years ago

Yes, it does still happen, even with Flipper.

Firebase: Error (auth/already-initialized).

sam-gc commented 2 years ago

The problem is that initializeAuth and getAuth can be called multiple times but only if the underlying dependencies don't change. For example, calling initializeAuth({persistence: browserLocalPersistence}); 10 times won't throw auth/already-initialized, but calling initializeAuth({persistence: browserLocalPersistence}); initializeAuth({persistence: browserSessionPersistence}); will throw the error since the persistence dependency has changed.

In this particular case, getReactNativePersistence returns a new object every time its called, so it appears initializeAuth is being called with different dependencies. It may be possible to avoid this behavior, but I can't give any promises as to timeline. In the meantime, you may be able to avoid this problem by disabling fast refresh specifically in the file that calls initializeAuth (e.g. https://github.com/vercel/next.js/issues/13268#issuecomment-743911710)

Nantris commented 2 years ago

Thanks for your reply @sam-gc.

Our file that calls initializeAuth has no React exports, so I think that the workaround you linked probably cannot work for us.

This is the full file that calls initializeAuth in our app looks like below, and the dependencies for initializeAuth shouldn't change - unless the firebaseApp argument is changed? firebaseApp is the output of initializeApp(config).

When the app is reloaded, initializeApp(config) presumably gets called anew and passed to initializeAuth:

import { initializeAuth } from 'firebase/auth';
import { getReactNativePersistence } from 'firebase/auth/react-native';
import AsyncStorage from '@react-native-async-storage/async-storage';

export const afterFirebaseInitialize = firebaseApp => {
  initializeAuth(firebaseApp, {
    persistence: getReactNativePersistence(AsyncStorage),
  });
};
sam-gc commented 2 years ago

When this gets reloaded, getReactNativePersistence(AsyncStorage) will return a different object -- that's where the issue lies. firebaseApp shouldn't be changing, since initializeApp similarly returns the same object for the same dependencies

Nantris commented 2 years ago

It shouldn't get reloaded as a new object in the code we have, but it could be - I assume must be if the only possible cause for this issue is initializeAuth being called anew. But like I said, it shouldn't be. We never unset firebaseApp once it's set.

It's hard to know for sure though because the issue doesn't occur 100% of the time now with Flipper as our debugging tool (as opposed to react-native-debugger) and the quirks of React Native debugging.

Our code looks like this:

let firebaseApp;

const makeFirebase = config => {
  if (firebaseApp) return firebaseApp;

  firebaseApp =
    typeof global.window !== 'undefined' ? initializeApp(config) : null;

  if (typeof afterFirebaseInitialize === 'function')
    afterFirebaseInitialize(firebaseApp);

  return firebaseApp;
};
sam-gc commented 2 years ago

That's right -- this error can only be thrown if initializeAuth is called twice, with different objects in the dependencies. Fixing this feature for React Native Fast Refresh is not on our roadmap at the moment, but we can keep this issue open to track the request

Nantris commented 2 years ago

@sam-gc why would the line if (firebaseApp) return firebaseApp; not prevent double initialization? I assume there's no code that would cause the results of initializeApp(config) to be cleared, and that's what's stored in firebaseApp.

Something doesn't add up unless there's more to the story here.

Nantris commented 2 years ago

This remains an issue and we're definitely not initializing anything twice. It's painstaking to be without Fast Refresh, and it's apparent we're not the only ones with the issue based on the emojis on @sam-gc's reply.

It only began with getReactNativePersistence and there is exactly one place that initializeAuth is being called, and that's where we're defining the persistence method.

We call initializeApp before initializeAuth, but we certainly don't call initializeAuth any more than once.

This is it. This is the sole place it's called. I have went over this back when I opened the issue and again now, and there's nothing on our end that's not being done according to the way recommended in the docs.

export const afterFirebaseInitialize = firebaseApp => {
  initializeAuth(firebaseApp, {
    persistence: getReactNativePersistence(AsyncStorage),
  });
};

I'd appreciate it so much if this was given some attention.


Our codebase is used on Electron and on React Native.


In this particular case, getReactNativePersistence returns a new object every time its called, so it appears initializeAuth is being called with different dependencies.

@sam-gc as you can see in oru code example, this is not the case unless again the problem relies with Firebase - or with AsyncStorage, but that's passed to getReactNativePersistence, which again is on Firebase's end.

ishmaelmoreno commented 2 years ago

I've avoided updating my version of firebase due to this issue. I hope it is resolved soon because it defeats the purpose of using react native.

Nantris commented 2 years ago

Friendly bump @sam-gc.

I hope this will not go neglected. It's hard for me to fathom fixing a newly introduced bug not being "on the roadmap." An undocumented regression without a workaround is a bug that needs fixing in my book.

Nantris commented 2 years ago

If it's true that the explanation for this is users double invoking initializeAuth or getReactNativePersistence then this code should work, but it doesn't:

import { initializeApp } from 'firebase/app';
import { initializeAuth } from 'firebase/auth';
import { getReactNativePersistence } from 'firebase/auth/react-native';
import AsyncStorage from '@react-native-async-storage/async-storage';

let initialized = false;

const persistence = {
persistence: getReactNativePersistence(AsyncStorage),
};

export const afterFirebaseInitialize = firebaseApp => {
if (initialized) return;
initialized = true;
initializeAuth(firebaseApp, persistence);
};

let firebaseApp;

const makeFirebase = config => {
if (firebaseApp) return firebaseApp;

firebaseApp = initializeApp(config);
afterFirebaseInitialize(firebaseApp);
};

The explanation that this is a React Fast Refresh problem would hold water if this didn't only affect Firebase on React Native. No other projects on any other platform break like this, and crucially, nor did Firebase used to break like this with version 8 and below.

Even more crucially, Firebase 9 used to work as intended for us. I'll have to go back and try to figure out more about why that may be - too late right now.

hsubox76 commented 2 years ago

Oh! I just notiiced you're importing from both firebase/auth and firebase/auth/react-native. I think the second is actually a full copy of the first with a few modifications for RN, so I think you want to do just:

import { initializeAuth, getReactNativePersistence } from 'firebase/auth/react-native';

and get rid of the firebase/auth line. I'm not sure if this will fix your issue but can you give a try and see if it does? If it does, of course it seems we have some docs work to address.

google-oss-bot commented 2 years ago

Hey @Slapbox. We need more information to resolve this issue but there hasn't been an update in 5 weekdays. I'm marking the issue as stale and if there are no new updates in the next 5 days I will close it automatically.

If you have more information that will help us get to the bottom of this, just add a comment!

ishmaelmoreno commented 2 years ago

Hi @Slapbox did you try @hsubox76's suggestion?

emmanuel-2u commented 2 years ago

Just stumbled upon this thread and I happen to have the same code as @hsubox76 suggested. I imported initializeAuth from firebase/auth/react-native but the problem persists. During fast refresh, auth/already-initialized error gets thrown.

hsubox76 commented 2 years ago

Can you show all of your relevant code, including all Firebase imports and where you call initializeApp and initializeAuth?

emmanuel-2u commented 2 years ago

Code

import { initializeApp } from 'firebase/app';
import AsyncStorage from '@react-native-async-storage/async-storage';
import {
    initializeAuth,
    getReactNativePersistence
} from 'firebase/auth/react-native';

// Replace with real values
const firebaseConfig = {
    apiKey: "apiKey",
    authDomain: "authDomain",
    projectId: "projectId",
    storageBucket: "storageBucket",
    messagingSenderId: "messagingSenderId",
    appId: "appId"
};

const firebaseApp = initializeApp(firebaseConfig);

const firebaseAuth = initializeAuth(firebaseApp, {
    persistence: getReactNativePersistence(AsyncStorage)
});
hsubox76 commented 2 years ago

I'm not totally sure how React Native fast refresh works but it seems like the other user in this thread had to put in some kind of check to make sure that initializeAuth() doesn't get called a second time on refresh, while the previous instance still exists in memory. The other user's code (from above) was:

let initialized = false;

const persistence = {
persistence: getReactNativePersistence(AsyncStorage),
};

export const afterFirebaseInitialize = firebaseApp => {
if (initialized) return;
initialized = true;
initializeAuth(firebaseApp, persistence);
};

Again I'm not sure what's needed for React Native fast refresh, or what it keeps or resets when it refreshes. If you wanted to test it you could put a console.log right next to initializeAuth() and see if it gets called again when you refresh, and if so, it would seem like initializeAuth() is getting called again on refresh, which you don't want.

Nantris commented 2 years ago

I haven't gotten a chance to try @hsubox76's suggestion because I'm pretty sure we can't use it. Our code is cross-platform, so I don't think we can easily implement the suggestion, but maybe via babel-module-resolver-plugin. I'll try to find time to test it in the near future.

emmanuel-2u commented 2 years ago

@hsubox76 I noticed the checks and I've tried it before commenting but it didn't work. I added the console.log and noticed it's getting called again.

However, this worked for me

import { getAuth, initializeAuth, getReactNativePersistence} from "firebase/auth/react-native";
import AsyncStorage from "@react-native-async-storage/async-storage";

if (getApps().length === 0) {
app = initializeApp(firebaseConfig);
auth = initializeAuth(app, {
persistence: getReactNativePersistence(AsyncStorage),
});

} else {
app = getApp();
auth = getAuth(app);
}

https://github.com/firebase/firebase-js-sdk/issues/6050#issuecomment-1119131377

hsubox76 commented 2 years ago

Sorry to hijack this thread but I'm trying to make sure we correct our documentation to be clear about how to use firebase/auth/react-native to prevent people from going through these headaches unnecessarily, and I don't seem to be able to find the documentation for it. Could anyone using that package tell me where you found out about firebase/auth/react-native? Was it just from Github issues or looking at the Firebase code itself, or is there a docs page somewhere I missed? If there aren't any docs I'll try to add some.

Nantris commented 2 years ago

@hsubox76 thanks for your continued attention on this issue.

Could anyone using that package tell me where you found out about firebase/auth/react-native?

I first became aware of it from comments on issue #6050, which do link to some mentions in the API reference - specifically here and in comments below.

tranlammedia commented 1 year ago

this í my sovle. use {initializeAuth} from 'firebase/auth/react-native' replace for {getAuth} from "firebase/auth";

import AsyncStorage from '@react-native-async-storage/async-storage';
import { initializeApp } from 'firebase/app';
import {
  initializeAuth,
  getReactNativePersistence
} from 'firebase/auth/react-native';

// add firebase config here

// initialize firebase app
const app = initializeApp(firebaseConfig);

// initialize auth
const auth = initializeAuth(app, {
  persistence: getReactNativePersistence(AsyncStorage)
});

export { auth };
martinivanalfonso commented 1 year ago

This seems to be fixed on version 10.0.0

thank you @hsubox76

Sidenote for anyone following this thread, remove this fix if implemented as it seems to break fast refreshing otherwise

const auth = initializeAuth(app, { persistence: getReactNativePersistence(AsyncStorage) });

hsubox76 commented 1 year ago

Thanks for the update. I'll close this issue for now and add a link to the release notes for 10.0.0 to make it clear that

1) We got rid of the special explicit path for the react-native bundle and added a react-native field to the package.json so that the react-native should be automatically chosen by the Metro bundler 2) The AsyncStorage community package is now a dependency of firebase/auth and should now be the default if you call getAuth() in react native

https://firebase.google.com/support/release-notes/js#version_1000_-_july_6_2023