app-types is a peerDependency requiring non-TS projects to install it manually.

[jahed]

[REQUIRED] Describe your environment

• Operating System version: Windows 10
• Firebase SDK version: 4.10.1
• Firebase Product: app, auth, database

[REQUIRED] Describe the problem

app-types is a peerDependency for all product packages (specifically, their individual -types package). For non-TypeScript projects, it seems unnecessary to require us to include another dependency into our packages when all it does is introduce TypeScript definitions.

Why not include it as an optionalDependency or even just a regular dependency so that non-TS users don't need to care about it?

Steps to reproduce:

Adding any package without app-types. e.g.

yarn add @firebase/database

Prints the following warnings:

warning "@firebase/database > @firebase/database-types@0.1.2" has unmet peer dependency "@firebase/app-types@^0.1.0".
warning " > @firebase/database@0.1.11" has unmet peer dependency "@firebase/app-types@^0.1.0".

[google-oss-bot]

Hey there! I couldn't figure out what this issue is about, so I've labeled it for a human to triage. Hang tight.

[jshcrowthe]

Hey @jahed, this issue is actually not a firebase issue but rather a yarn issue. If you try doing the following, you should see no issues:

npm install @firebase/database @firebase/app

That said, the warnings you are seeing from yarn, are actually false positives and the code will work just fine.

[jahed]

Surely then if the code works fine without it, it's an optional dependency and not a peer dependency?

…

On Fri, 2 Mar 2018, 17:13 Josh Crowther, ***@***.***> wrote: Closed #541 <#541>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#541 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADLTk1i3n0IYU3COLVZNAtOJNeuhWNi8ks5taX3LgaJpZM4SY4fs> .

[jshcrowthe]

Not quite:

That said, the warnings you are seeing from yarn, are actually false positives and the code will work just fine.

The code will not compile for TS users without those deps. So it is not an optionalDependency as you suggest. For JS only users (i.e. you aren't using the provided typings at all, including through VSCode or one of the Typings extensions in other editors) it is completely negligible, but it is a use case we support.

The point I was making was that the issue here is yarn mishandling peer deps. See yarnpkg/yarn#4850 for more info here.

However, the package will function as expected even with the errors as the correct packages are installed.

[jahed]

Why not make it a regular dependency rather than a peer dependency if you need TS support? That's how most packages are published when written in Flow or TS. The issue linked isn't related to this as I did install app-types and the warning did go away. The issue in that ticket is about warnings even when they are installed. And when using npm, I can see the same warning. The npm documentation also mentions this.

UPDATE: npm versions 1 and 2 will automatically install peerDependencies if they are not explicitly depended upon higher in the dependency tree. For all following versions of npm (starting with ***@***.***), this will no longer be the case. You will receive a warning that the peerDependency is not installed instead.

My point is why do I need to install app-types explicitly in my own project if I don't need it? Peer Dependencies are for entirely different use cases than what it's being used for here. It's for plugins and add-ons. https://nodejs.org/en/blog/npm/peer-dependencies/

…

On Fri, 2 Mar 2018, 17:30 Josh Crowther, ***@***.***> wrote: Not quite: That said, the warnings you are seeing from yarn, are actually *false positives* and the code will work just fine. The code *will not compile* for TS users without those deps. So it is not an optionalDependency as you suggest. For JS only users (i.e. you aren't using the provided typings at all, including through VSCode or one of the Typings extensions in other editors) it is completely negligible, but it is a use case we support. The point I was making was that the issue here is *yarn* mishandling peer deps. See yarnpkg/yarn#4850 <yarnpkg/yarn#4850> for more info here — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#541 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADLTkwhZIqG8tfM1XlwldJNk-hl1EPdaks5taYGcgaJpZM4SY4fs> .

[jshcrowthe]

I may be misunderstanding here a bit.

Are you installing @firebase/app in your app?

[jahed]

Apologies, the workaround is straight forward so I thought I'd mention the issue and see what the team thought.

I'll bring this issue back up once yarn yarnpkg/yarn#4850 is fixed, as until then it'll be a point of contention.

[kevmo314]

Hi, yarnpkg/yarn#4850 was closed and the yarn team indicated that this is not something they can act on. Can the firebase team either follow up or fix the dependency structure?

[theophanemayaud]

I have the same problem, and found the same fix : manually installing @firebase/app-types. But since it literally states on the npm package page :

This package is not intended for direct usage, and should only be used via the officially supported firebase package.

I think we should find a way to fix these unsettling warnings... !