Fixing the types for firebase/app

[jamesdaniels]

import firebase from 'firebase/app'; // Does not work (it should however)

import * as firebase from 'firebase/app'; // Compiles but fails w/runtime errors

Closing #1184 in favor of this... While my heart was in the right place (in terms of firebase/app) I was modifying the types for firebase and introducing breaking changes.

Let's start fresh shall we, the root cause is that the typings for firebase/app are pointing to that of firebase. Which is not correct.

firebase/app is an ESM+CJS with a default export, the types as defined in the root use export = ... which suggests AMD and does not play nice with ESM.

This fix simply reexports firebase from .. in a new index.d.ts.

[jamesdaniels]

FYI @davideast

[jamesdaniels]

@jshcrowthe this will break anyone pulling in the namespaces from firebase/app, like so:

import { default as firebase, auth, database } from 'firebase/app';

Are we ok with that? If not, we could fix with the following lines in app/index.d.ts:

export import auth = firebase.auth;
export import database = firebase.database;
export import firestore = firebase.firestore;
// and so on for all the exported types

This quick export wouldn't address them being exported as both namespace and callable however, which would be poor typing (e.g, import { auth } from 'firebase/app'; auth() would compile but obviously error when run). Though this bug exists today and we could consider out of scope.

That said, I don't know if exporting the namespaces is worth it however; as a quick skim of Github search results (not thorough/scientific) shows more people importing types from @firebase/app (which isn't stable) then from firebase/app.

Thoughts?

[jamesdaniels]

Alternatively we could do the reverse; move the types I did in #1184 to app/index.d.ts and then change index.d.ts to:

import * as firebase from 'app';
export = firebase;
export as namespace firebase;

[Feiyang1]

@jamesdaniels Wouldn't the change break import * as firebase from 'firebase/app', unless we reexport everything from firebase namespace in firebase/app? Maybe we can migrate the index.d.ts file at the root of firebase to esm with both named and default exports? Is there any downside to this approach? I'm probably missing something, but I'm not sure why we chose cjs format for the root index.d.ts.

[jamesdaniels]

@Feiyang1 import * as firebase from 'firebase/app' is already broken; that's actually the problem. It passes lint but fails with runtime errors if you're targeting Node.
You can safelyimport * as firebase from 'firebase', not firebase/app.

import firebase from 'firebase/app' is the correct syntax, but doesn't pass lint due to our manual types being incorrect.

[googlebot]

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.