Closed AviVahl closed 1 year ago
@hsubox76 Is this related to https://github.com/firebase/firebase-js-sdk/pull/7128?
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).
npm@9 installs peerdeps by default so this actually ends up installing react-native for end users
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
.
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.
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.
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.
I'm hesitant about making it an optional peerDependency because I've seen issue reports like this NextJS issue: https://github.com/vercel/next.js/issues/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.
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.
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
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 .
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
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