Add @firebase/logger as dependency to app-types

[DennisSimon]

Hey everyone,

i ran into an issue with the app-types module where the typescript compilation of a module that defined firebase-admin as a dependency failed with the following error:

.yarn/cache/@firebase-app-types-npm-0.6.2-936c84268d-243af1c06f.zip/node_modules/@firebase/app-types/index.d.ts:17:57 - error TS2307: Cannot find module '@firebase/logger' or its corresponding type declarations.

The project uses yarn2 with a 'strict' PnP mode.

I have created a small codesandbox that demonstrates this issue: https://codesandbox.io/s/gallant-satoshi-fycm1
Run yarn compile to see the issue in action.

Looking forward to your review!

[changeset-bot]

🦋 Changeset detected

Latest commit: e7f7cc9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@firebase/app-types	Patch
@firebase/app	Patch
@firebase/database-types	Patch
@firebase/database	Patch
firebase	Patch
@firebase/rules-unit-testing	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[google-cla]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[DennisSimon]

@googlebot I signed it!

[Feiyang1]

Thanks for the contribution! It should be a peerDependencies instead. Can you please make the change?

[DennisSimon]

Hi @Feiyang1 , thanks for the comment!
I have one question regarding this change:
So from my understanding, marking this as a peerDependency tells node that it should look for the module somewhere else in the dependency-tree, as it should already be installed.
Yet in my case @firebase/logger is not included anywhere in the dependency-tree of firebase-admin and will therefore not be found even when adding it as a peer dependency.

In this case, would i still add @firebase/logger as a peerDependency here and should i then add @firebase/logger as a dependency in firebase-admin?

[Feiyang1]

@firebase/logger is a dependency of @firebase/database, so it should be installed.
However, yarn pnp mode has stricter rules that prevent packages to require dependencies that they don't list explicitly, so I think that's why you are seeing the error.

Is there an easy way we can test it locally?

[DennisSimon]

So i tested it by adding the following entry to my .yarnrc.yaml file:

packageExtensions:
  "@firebase/app-types@*":
    peerDependencies:
      "@firebase/logger": "0.x"

After running yarn install, i got the following message:

➤ YN0002: │ @firebase/database-types@npm:0.7.2 doesn't provide @firebase/logger (pbcca2), requested by @firebase/app-types

So it looks like yarn is searching for peer-dependencies of @firebase/app-types only one level up?

After extending my .yarnrc.yaml and declaring @firebase/logger as a dependency of @firebase/database-types, the peer dependency is resolved and the compilation works.

packageExtensions:
  "@firebase/database-types@*":
    dependencies:
      "@firebase/logger": "0.x"
  "@firebase/app-types@*":
    peerDependencies:
      "@firebase/logger": "0.x"

I'll look into it more tomorrow but at this point i think it is also possible that this might also be a bug of yarn.

I will change the dependency type to a peerDependency.

[Feiyang1]

So it looks like yarn is searching for peer-dependencies of @firebase/app-types only one level up?

It kinda makes sense. The peerDependencies of package A should be provided by packages that directly depend on A.

So I think we may need to make @firebase/logger a dependency of @firebase/app-types, and let's use 0.2.6 instead of 0.x.

[DennisSimon]

Looking at the code of yarn here it seems like it tries to follow the tree up.
I think that would also make sense for projects like firebase that exist (for the most of it) in a monorepo.
Well, i can check with yarn if this is expected behaviour or if its a bug but for now i agree that we can add @firebase/logger as a dependency of @firebase/app-types.

[Feiyang1]

SG. Can you please add a changeset to this PR via the link in #5144 (comment) ?

[DennisSimon]

Sure thing, added the changeset.

[DennisSimon]

Hi @Feiyang1, quick ping as a reminder that the changeset is added