Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FR: Make firebase compatible with Yarn2 pnp #3707

Open
ScottPierce opened this issue Aug 30, 2020 · 10 comments · May be fixed by #4011
Open

FR: Make firebase compatible with Yarn2 pnp #3707

ScottPierce opened this issue Aug 30, 2020 · 10 comments · May be fixed by #4011

Comments

@ScottPierce
Copy link

ScottPierce commented Aug 30, 2020

[REQUIRED] Describe the problem

I get an error when bundling with Yarn 2 and firebase:

(node:82642) [MODULE_NOT_FOUND] Error: @firebase/database tried to access @firebase/app, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Steps to reproduce:

I'm building a create-react-app application with yarn2 with pnp that depends on firebase, and it outputs the following error:

(node:82642) [MODULE_NOT_FOUND] Error: @firebase/database tried to access @firebase/app, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.
@google-oss-bot
Copy link
Contributor

I found a few problems with this issue:

  • I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.
  • This issue does not have all the information required by the template. Looks like you forgot to fill out some sections. Please update the issue with more information.

@Feiyang1
Copy link
Member

Thanks for the report! Are you able to get it to work using loose mode (I tried it, but it still didn't work)? Otherwise I'm afraid firebase is not compatible with yarn 2 pnp at the moment.

As the message states, @firebase/app is not declared as a dependency for @firebase/database. This is for working around the unmet peerDependency warning for firebase-admin users (see this PR). I admit it is not an ideal solution and we probably should create a separate npm package to be used in firebase-admin, so we can list dependencies correctly.

I will keep it open as a feature request, but it is probably a low priority for us right now.
@hiranya911 FYI

@Feiyang1 Feiyang1 changed the title Error: @firebase/database tried to access @firebase/app, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound. FR: Make firebase compatible with Yarn2 pnp Aug 31, 2020
@PutziSan
Copy link

PutziSan commented Oct 15, 2020

This problem is still there.

Currently workaround is adding this to .yarnrc.yml

packageExtensions:
  # https://github.com/firebase/firebase-js-sdk/issues/3707
  "@firebase/database@*":
    peerDependencies:
      "@firebase/app": "*"
      "@firebase/app-types": "*"

info about packageExtensions:

Some packages may have been specified incorrectly with regard to their dependencies - for example with one dependency being missing, causing Yarn to refuse it the access. The packageExtensions fields offer a way to extend the existing package definitions with additional information.

https://yarnpkg.com/configuration/yarnrc#packageExtensions

@Zloka
Copy link

Zloka commented Aug 16, 2021

Thanks for the report! Are you able to get it to work using loose mode (I tried it, but it still didn't work)? Otherwise I'm afraid firebase is not compatible with yarn 2 pnp at the moment.

As the message states, @firebase/app is not declared as a dependency for @firebase/database. This is for working around the unmet peerDependency warning for firebase-admin users (see this PR). I admit it is not an ideal solution and we probably should create a separate npm package to be used in firebase-admin, so we can list dependencies correctly.

I will keep it open as a feature request, but it is probably a low priority for us right now.
@hiranya911 FYI

@Feiyang1 not sure what the correct place and practice for bumping feature requests is, and I understand the low priority, but for what it is worth our team would like to make use of Yarn PnP as well, and ended up here 🙂

@mikob
Copy link

mikob commented Apr 13, 2022

This problem is still there.

Currently workaround is adding this to .yarnrc.yml

packageExtensions:
  # https://github.com/firebase/firebase-js-sdk/issues/3707
  "@firebase/database@*":
    peerDependencies:
      "@firebase/app": "*"
      "@firebase/app-types": "*"

info about packageExtensions:

Some packages may have been specified incorrectly with regard to their dependencies - for example with one dependency being missing, causing Yarn to refuse it the access. The packageExtensions fields offer a way to extend the existing package definitions with additional information.

https://yarnpkg.com/configuration/yarnrc#packageExtensions

Thank you @PutziSan for the headstart. I needed the following for firebase v9:

  "@firebase/auth@*":
    dependencies:
      "@firebase/app": "*"
      "@firebase/app-types": "*"
  "firebase@*":
    dependencies:
      "@firebase/auth": "*"
      "@firebase/app": "*"
  "@lipsurf/extension@*":  # the project that uses firebase
    dependencies:
      "@vue/runtime-core": "*"
      "@firebase/auth": "*"
      "@firebase/app": "*"
      "@firebase/app-types": "*"

@hsubox76
Copy link
Contributor

Revisiting this, looks like the error text is now something like:

Module not found: Error: Can't resolve '@firebase/app' in '/Users/chholland/bug-repos/pnp/.yarn/cache/@firebase-database-npm-0.14.0-bfea20510f-099a31491e.zip/node_modules/@firebase/database/dist'

The same fix (packageExtensions) works. It's a little hard to test if moving @firebase/app to dependencies in firebase/database/package.json would fix it because I'm not really sure how to load a local development version of a package into Yarn pnp. I am guessing it would.

In that case the only real fix is to make a separate @firebase/database package for admin (it's shared by the JS SDK and the Admin SDK, and the JS SDK version should have @firebase/app as a dependency and the admin version shouldn't). We have been trying to compromise between the needs of the 2 consuming SDKs and with Yarn PNP and PNPM's requirements it seems like the compromise is no longer possible and we need an exclusive JS SDK version with @firebase/app as a real dependency.

I think this will be a little easier after Admin migrates to consuming database instead of database-compat (which transitively depends on database) because then we can just make 1 additional package instead of 2.

@jonaldinger
Copy link

In case this is helpful to anyone else, this is what I needed in my .yarnrc.yml to silence all of the firebase dependency warnings from Yarn v3.5.0 in PnP strict mode (the default). I'm only using the auth portion of firebase, so the code functions with a much smaller set of declarations, but seeing a dozen warnings on every yarn install for things that aren't actually broken makes it very hard to notice when a new/actually meaningful warning from any other dependency is emitted.

packageExtensions:
  '@firebase/analytics@*':
    dependencies:
      '@firebase/app': '*'
  '@firebase/app-check@*':
    dependencies:
      '@firebase/app': '*'
  '@firebase/auth@*':
    dependencies:
      '@firebase/app': '*'
      '@firebase/app-types': '*'
  '@firebase/auth-types@*':
    dependencies:
      '@firebase/app-types': '*'
  '@firebase/firestore@*':
    dependencies:
      '@firebase/app': '*'
  '@firebase/firestore-types@*':
    dependencies:
      '@firebase/app-types': '*'
  '@firebase/functions@*':
    dependencies:
      '@firebase/app': '*'
  '@firebase/installations@*':
    dependencies:
      '@firebase/app': '*'
  '@firebase/installations-types@*':
    dependencies:
      '@firebase/app-types': '*'
  '@firebase/messaging@*':
    dependencies:
      '@firebase/app': '*'
  '@firebase/performance@*':
    dependencies:
      '@firebase/app': '*'
  '@firebase/remote-config@*':
    dependencies:
      '@firebase/app': '*'
  '@firebase/storage@*':
    dependencies:
      '@firebase/app': '*'
  '@firebase/storage-types@*':
    dependencies:
      '@firebase/app-types': '*'

@hsubox76
Copy link
Contributor

What's the warning message you're seeing? Skimming through it looks like these packages have @firebase/app listed in peerDependencies, is that not okay for Yarn PnP? It would have to be moved to dependencies? Just wondering why peerDependencies isn't working

@jonaldinger
Copy link

Here is the Yarn output when I don't manually declare package extensions (ignore the redux alpha warning at the bottom):

yarn-output

Then if I run yarn explain peer-requirements with the hash from one of those specific firebase dependencies throwing the warning, such as @firebase/analytics-compat (p6d576), I get:

peer-requirements

So I'm not sure exactly what's going on because I'm not familiar with the "compat" library situation, but it seems like the source of the issue is emanating from the compat libraries. This is reinforced by the fact that if I manually install @firebase/app in my project dependencies, I still see all the warnings. Based on the warning message, my guesses for the realm of potential solutions would be:

  • In the non-compat libraries (such as @firebase/analytics):
    1. Change the declaration of @firebase/app from peer + dev dependency to a dependency
  • In the compat libraries (such as @firebase/analytics-compat):
    1. Add @firebase/app as a peer dependency (along side of the existing @firebase/app-compat peer dependency)
    2. Add @firebase/app as a dependency
    3. Change the declaration of @firebase/app-compat from peer + dev dependency to a dependency (because @firebase/app-compat does declare @firebase/app as a dependency internally)

Because I'm not familiar with the compat library architecture and how things actually depend on each other, I don't know the broader implications of these suggestions, their effects on tree shaking etc. Just trying to provide as much information as I can.

@hsubox76
Copy link
Contributor

hsubox76 commented May 3, 2023

Got it, thanks so much for the explanation. I think I understand the requirements now and will try to figure out the best way to address them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants