Regression (v10): fresh firebase installation has peerDep error on react-native?

[AviVahl]

Operating System

Fedora 38

Browser Version

N/A

Firebase SDK Version

10.1.0

Firebase SDK Product:

Irrelevant

Describe your project's tooling

Irrelevant

Describe the problem

Saw this after upgrading from v9 to v10.1.0 (now that firebaseui is compatible)
Upon installation, the following peerDep warning appears:
warning "firebase > @firebase/auth > @react-native-async-storage/async-storage@1.19.1" has unmet peer dependency "react-native@^0.0.0-0 || 0.60 - 0.72 || 1000.0.0".

Steps and code to reproduce issue

mkdir firebase-test
cd firebase-test
yarn init -y
yarn add firebase

[pragatimodi]

@hsubox76 Is this related to #7128?

[hsubox76]

Yes, so see that issue for why we added @react-native-async-storage/async-storage as an auth dependency - the React Native version of Auth requires it. This warning won't pop up for React Native users because they'll have the react-native dep (a transitive peerDependency of the async-storage package) installed. Unfortunately it will pop up for non-React Native users, which is the majority.

Fortunately, it's just a warning and shouldn't block development. I do think it's a bad situation for a React-Native-related warning to impact the majority of auth users who have nothing to do with React Native, and we'll do our best to try to find a way to remove it.

I can't currently think of a way, however. We could convert the @react-native-async-storage/async-storage package from a dependency to a peerDependency in our own package.json but that would just give the same warning on install, where the unmet peerDependency is now that package. I think the only solution would be separate top-level package.jsons for the RN bundle vs the main auth bundle, but that would require creating a new NPM package (since this is an npm/yarn install warning and not a module resolution warning, which can use nested package.jsons).

[AviVahl]

npm@9 installs peerdeps by default so this actually ends up installing react-native for end users

[gavinsharp]

npm@9 installs peerdeps by default so this actually ends up installing react-native for end users

And indeed this broke various things in our monorepo when upgrading, since react-native in turn pulled in a conflicting version of react.

[hsubox76]

I'm not sure how to solve this except for going back to the old way of requiring RN users to manually import and include @react-native-async-storage/async-storage which is technically a breaking change for anyone who's already converted to getAuth(), but since we just implemented the new way a month ago, it might not be too late to revert it. I think we may have to do this because it's not worth breaking non-RN users (the majority) for ease of use of RN users.

[AviVahl]

Make it a peerDependency and mark it as optional: https://docs.npmjs.com/cli/v9/configuring-npm/package-json#peerdependenciesmeta
This appears to not install it by default, but still allow you to define a supported semver range.
Tested with node-fetch@2, which has an optional peerDependency on "encoding".
Both npm@9 and yarn@1 did not install, or warn about it.

[hsubox76]

Sure, but that's still going to be a "breaking change" for any RN users who are currently using getAuth(), as they'll have to manually install @react-native-async-storage/async-storage as a dependency in their own project. I will probably add a clear error message for RN users who call getAuth() or who call initializeAuth() without providing a persistence option, to tell them to install it.

Before making it a peerDependency (as opposed to removing it as a dependency altogether) I want to double check it won't cause a problem for pnpm or yarn 2/3 or any other package installers. I know pnpm can be very strict.

[hsubox76]

I'm hesitant about making it an optional peerDependency because I've seen issue reports like this NextJS issue: vercel/next.js#7621 where users are still getting a warning on an optional peerDependency and aren't happy about it. I think we may just have to remove it altogether and rely on the warning/documentation to tell RN users to add it.

[hsubox76]

I did some more investigation and found that the NextJS "encoding" issue is not related to package installers or the package.json in any way and is caused by node-fetch having a conditional require("encoding") in its code. Looks like all the major package managers support peerDependenciesMeta optional flag, so I'm adding it to the PR.

[hsubox76]

It's been removed as a dependency in v10.3.0 and added as an optional peerDependency using peerDependenciesMeta. Let me know if you're still seeing issues related to this dependency and I'll reopen this issue.

See the release notes for how this change affects React Native users: https://firebase.google.com/support/release-notes/js#version_1030_-_august_22_2023

[KeunwooPark]

Hi. I'm coming from this release note.
I want to share that firebase@10.3.0 does not solve the problem of importing getReactNativePersistence.
Here is my question in Stack Overflow. I was using firebase@10.1.0 at that time and I upgraded to firebase@10.3.0, but still seeing the issue. It seems like others are also facing this issue(ref).

I'm using Expo@48 and react-native@0.71.8 .

[Nantris]

Just upgraded to 10.4 from 9.23 and our app crashes now due to some error in Firebase auth. Unfortunately Hermes gives such crappy error messages that that's really all I can say. We get: [TypeError: undefined is not a function]

We're using the Auth as specified here: https://firebase.google.com/support/release-notes/js#authentication

For what it's worth we use Yarn 1.x