[Firestore] firebase-admin doesn't work without optional dependencies

mbelsky commented 4 years ago

Hi,

My app throws MODULE_NOT_FOUND if I install project's dependencies without optional packages.

[REQUIRED] Step 2: Describe your environment

Operating System version: macOS 10.15.17, alpine:3.12
Firebase SDK version: 9.4.1
Firebase Product: Firestore
Node.js version: 14.15
NPM version: 6.14.8

[REQUIRED] Step 3: Describe the problem

Steps to reproduce:

git clone https://github.com/mbelsky/triage.git
cd triage && git co firebase-admin-node
Copy firestore's cert.json in the triage directory
npm run repro: this command installs project's dependencies without optional and runs firestore.js.

Output:

> rm -rf ./node_modules && npm i --no-optional && npm start

npm WARN triage@1.0.0 No description
npm WARN triage@1.0.0 No repository field.

added 32 packages from 59 contributors and audited 156 packages in 1.363s

1 package is looking for funding
  run `npm fund` for details

found 0 vulnerabilities

> triage@1.0.0 start /Users/mbelsky/gh/triage
> FIREBASE_CONFIG=./cert.json node ./firestore.js

internal/modules/cjs/loader.js:883
  throw err;
  ^

Error: Cannot find module '@google-cloud/firestore'
Require stack:
- /Users/mbelsky/gh/triage/node_modules/firebase-admin/lib/firebase-namespace.js
- /Users/mbelsky/gh/triage/node_modules/firebase-admin/lib/default-namespace.js
- /Users/mbelsky/gh/triage/node_modules/firebase-admin/lib/index.js
- /Users/mbelsky/gh/triage/firestore.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)
    at Function.Module._load (internal/modules/cjs/loader.js:725:27)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at FirebaseNamespace.get (/Users/mbelsky/gh/triage/node_modules/firebase-admin/lib/firebase-namespace.js:306:29)
    at Object.<anonymous> (/Users/mbelsky/gh/triage/firestore.js:9:18)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/Users/mbelsky/gh/triage/node_modules/firebase-admin/lib/firebase-namespace.js',
    '/Users/mbelsky/gh/triage/node_modules/firebase-admin/lib/default-namespace.js',
    '/Users/mbelsky/gh/triage/node_modules/firebase-admin/lib/index.js',
    '/Users/mbelsky/gh/triage/firestore.js'
  ]
}
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! triage@1.0.0 start: `FIREBASE_CONFIG=./cert.json node ./firestore.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the triage@1.0.0 start script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/mbelsky/.npm/_logs/2020-11-21T23_23_24_210Z-debug.log
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! triage@1.0.0 repro: `rm -rf ./node_modules && npm i --no-optional && npm start`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the triage@1.0.0 repro script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/mbelsky/.npm/_logs/2020-11-21T23_23_24_245Z-debug.log

Relevant Code:

https://github.com/mbelsky/triage/blob/addf8315b754f5657987b4a13ac11def29728131/firestore.js#L1

As I see this happens bcs this line and @google-cloud/firestore defined as optional deps.

Should the dependency be defined as not optional?

google-oss-bot commented 4 years ago

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 seem to follow the issue template. Make sure you provide all the required information.

hiranya911 commented 4 years ago

Dependency is required if you're using Firestore APIs, which your code does. Our Firestore support is provided by the @google-cloud/firestore library. So if your code calls admin.firestore(), consider that dependency required.

We have declared it as optional because we have seen random installation errors with those dependencies in the past, and in general we don't want the installation of firebase-admin to fail because of them.

AlvesJorge commented 3 months ago

We have declared it as optional because we have seen random installation errors with those dependencies in the past, and in general we don't want the installation of firebase-admin to fail because of them.

This makes no sense. Your package does not work without firestore and storage so you need to require these as non optional dependencies. That's what a dependency is.

It's like planning a Bow & Arrow competition in Antartica, but saying that bringing a Bow is optional because some people couldn't get it through airport security in the past. You make us think we don't need the bow.

we have seen random installation errors

I'd rather my deps fail to install and not work, than it successfully install but not work.

firebase / firebase-admin-node