FR: Make Firebase SDK compatible with Angular 10

[naveedahmed1]

Describe your environment

• Operating System version: All
• Browser version: All browsers
• Firebase SDK version: 7.15.5
• Firebase Product: All

Describe the problem

This issue is related to angular/angularfire#2478 reported in angularfire repo.

Google advises against using CommonJS modules because it can impact the tree-shaking of application as explained here https://web.dev/commonjs-larger-bundles/

I would therefore request you to please release this plugin as es2015 module so that the production builds are properly tree-shaken.

Thank you

[akauppi]

@naveedahmed1 Your issue is interesting to me, since I'm working on a Firebase web app that only uses ES modules.

I think it already is:

$ ls -al node_modules/firebase/app/dist/
total 32
drwxr-xr-x  6 asko  staff  192 27 Kes 19:21 .
drwxr-xr-x  4 asko  staff  128 27 Kes 19:21 ..
-rw-r--r--  1 asko  staff  973 26 Lok  1985 index.cjs.js
-rw-r--r--  1 asko  staff  991 26 Lok  1985 index.cjs.js.map
-rw-r--r--  1 asko  staff  835 26 Lok  1985 index.esm.js
-rw-r--r--  1 asko  staff  984 26 Lok  1985 index.esm.js.map

I'm using Vite for development. It picks the index.esm.js and I should be able to import Firebase as:

import * as firebase from 'firebase/app'
import 'firebase/auth'

However, here is where my problems start. The above leaves firebase.initializeApp undefined, and I don't get Firebase running.

This works:

import firebase from 'firebase/app'
import 'firebase/auth'

..but if I do so, I cannot take Firebase UI from npm. It presumably tries to find Firebase using the recommended loading (first).

---

Firebase 7.15.5
Node.js 14.4.0
Vite 1.0.0-beta.11

[akauppi]

@naveedahmed1 I notice you changed the title.

Would you care to shed more light, why you think Firebase is not "released as ES2015 module"?

My problems are already explained here: firebase/firebaseui-web#612

Firebase employees: please help keeping the issues tidy.

[swftvsn]

The angularfire ticket explains it in more detail:

This (angularfire) library is released as es2015 and abides by APF, however Firebase JS SDK does not abide by APF. The JS SDK's main package is a CJS whereas APF expects it to be a UMD. We can't change that until at least the next major. Please raise any issues related to the JS SDK over here (link points to this repo).

I'm no expert in this field, but it does sound like this lib has to change something to be able to conform to the Angular way of doing stuff.

APF (Angular Package Format) is explained here: https://docs.google.com/document/d/1CZC2rcpxffTDfRDs6p1cfbmKNLA6x5O-NtkJglDaBVs/edit

It expects UMD instead of CJS (or commonJS) as James Daniels (Whose also a Firebase engineer) explains in the comment: angular/angularfire#2478 (comment)

But true, maybe the title should be FR: Release as APF compatible module.

[abcfoundry]

We are also attempting to import into an Nativescript-angular application with the same errors as above. We have tried to use the ngcc file as a work around with no such luck. Any movement to move towards UMD would be much appreciated as @jamesdaniels suggests.

[abcfoundry]

It would be interesting to hear some thoughts from the core team if this is possible? If this makes sense? If this is in the real of possibilities ? If this is something that can be inplace for next release? Or is this just a dream and we should search out a work around?
Thoughts?

[hsubox76]

Changed the title of the issue for clarity.

To sum up the state of the issue so far:

1. We do provide an ES2015 module, but it is not exported as the "main" field in package.json.
2. This could be fixed by changing the "main" field to point to the ES2015 module, but this would break Node users that require CJS or UMD in that field, and this SDK needs to support Node.
3. This could possibly also be fixed by making the CJS bundle UMD, we are exploring this but haven't gotten a successful version that fixes the problem.
4. This could also be fixed by using the ngcc.config to look at another field in package.json but this causes type errors because of shared d.ts files. We will also look into if anything can be done about the d.ts files.

[Splaktar]

Here are the errors that I am seeing:

WARNING in /node_modules/@angular/fire/__ivy_ngcc__/fesm2015/angular-fire.js depends on 'firebase/app'. CommonJS or AMD dependencies can cause optimization bailouts.
For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies

WARNING in /node_modules/firebase/auth/dist/index.esm.js depends on '@firebase/auth'. CommonJS or AMD dependencies can cause optimization bailouts.
For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies

WARNING in /node_modules/firebase/app/dist/index.cjs.js depends on '@firebase/app'. CommonJS or AMD dependencies can cause optimization bailouts.
For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies

WARNING in /node_modules/firebase/performance/dist/index.esm.js depends on '@firebase/performance'. CommonJS or AMD dependencies can cause optimization bailouts.
For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies

This error message from Angular points to this docs page:
https://angular.io/guide/build#configuring-commonjs-dependencies

I think that the Angular CLI is actually looking for the fesm2015:, es2015:, and module: fields in the package.json. It wants an FESM (Flat ES Module) or an ES Module. All of the packages and sub-packages for Firebase appear to define a module: field, which should generally be "good enough".

However, I think that it falls back to main: if it can't find what it needs in the other fields. It appears that this might be happening due to the esm.js files not being properly generated/formatted.

The repo is on a recent version of Rollup 2.23.0. The latest is 2.23.1.

However, this repo is using

firebase-js-sdk/packages/firebase/package.json

Line 67 in 5b87b59

"rollup-plugin-node-resolve": "5.2.0",

From https://www.npmjs.com/package/rollup-plugin-node-resolve:

As that warning states, the package has been renamed to @rollup/plugin-node-resolve and it's now on version 8.4.0:
https://github.com/rollup/plugins/blob/e4d21bacd6c91b041ac4dda4a6c918196899222c/packages/node-resolve/package.json#L2-L3

8.4.0 was released just a month ago, while the 5.2.0 version was released 14 months ago.

Example

If I change the import in /node_modules/@angular/fire/__ivy_ngcc__/fesm2015/angular-fire.js from

import { apps, initializeApp, registerVersion } from 'firebase/app';

To:

import { apps, initializeApp, registerVersion } from 'firebase/app/dist/index.esm.js';

That file contains:

import firebase from '@firebase/app';
export { default } from '@firebase/app';

var name = "firebase";
var version = "7.17.1";

/**
 * @license
 * Copyright 2018 Google LLC
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *   http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
firebase.registerVersion(name, version, 'app');
//# sourceMappingURL=index.esm.js.map

The build gives this error:

"export 'apps' was not found in 'firebase/app/dist/index.esm.js'
    at HarmonyImportSpecifierDependency._getErrors (/node_modules/webpack/lib/dependencies/HarmonyImportSpecifierDependency.js:109:11)
    at HarmonyImportSpecifierDependency.getErrors (/node_modules/webpack/lib/dependencies/HarmonyImportSpecifierDependency.js:68:16)
    at Compilation.reportDependencyErrorsAndWarnings (/node_modules/webpack/lib/Compilation.js:1463:22)
    at /node_modules/webpack/lib/Compilation.js:1258:10
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:40:1)
    at AsyncSeriesHook.lazyCompileHook (/node_modules/tapable/lib/Hook.js:154:20)
    at Compilation.finish (/node_modules/webpack/lib/Compilation.js:1253:28)
    at /node_modules/webpack/lib/Compiler.js:672:17
    at _done (eval at create (/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:7:1)
    at eval (eval at create (/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:106:22)
    at /node_modules/webpack/lib/Compilation.js:1185:12
    at /node_modules/webpack/lib/Compilation.js:1097:9
    at processTicksAndRejections (internal/process/task_queues.js:79:11)
 @ ./src/app/app.module.ts
 @ ./src/main.ts
 @ multi ./src/main.ts

"export 'initializeApp' was not found in 'firebase/app/dist/index.esm.js'
...
"export 'registerVersion' was not found in 'firebase/app/dist/index.esm.js'
...
"export 'registerVersion' was not found in 'firebase/app/dist/index.esm.js'
...

---

PS. The rollup-plugin-commonjs package (10.1.0 released a year ago) is also deprecated, but being used by this repo. @rollup/plugin-commonjs is the new package and it's on 14.0.0 (published last month) now.

[Splaktar]

On more piece of possibly confusing data...

If I change it to the following bogus import:

import { apps, initializeApp, registerVersion } from 'firebase/app/joe';

I get these errors:

ERROR in ./node_modules/@angular/fire/__ivy_ngcc__/fesm2015/angular-fire.js
Module not found: Error: Can't resolve 'firebase/app/joe' in '/node_modules/@angular/fire/__ivy_ngcc__/fesm2015'
resolve 'firebase/app/joe' in '/node_modules/@angular/fire/__ivy_ngcc__/fesm2015'
  Parsed request is a module
  using description file: /node_modules/@angular/fire/package.json (relative path: ./__ivy_ngcc__/fesm2015)
    Field 'browser' doesn't contain a valid alias configuration
    resolve as module
      looking for modules in 
        using description file: /package.json (relative path: .)
          Field 'browser' doesn't contain a valid alias configuration
          using description file: /package.json (relative path: ./firebase/app/joe)
            no extension
              Field 'browser' doesn't contain a valid alias configuration
              /firebase/app/joe doesn't exist
            .ts
              Field 'browser' doesn't contain a valid alias configuration
              /firebase/app/joe.ts doesn't exist
            .tsx
              Field 'browser' doesn't contain a valid alias configuration
              /firebase/app/joe.tsx doesn't exist
            .mjs
              Field 'browser' doesn't contain a valid alias configuration
              /firebase/app/joe.mjs doesn't exist
            .js
              Field 'browser' doesn't contain a valid alias configuration
              /firebase/app/joe.js doesn't exist
            as directory
              /firebase/app/joe doesn't exist

From that, it appears to be looking at the browser: field in the package.json which all Firebase JS packages use to point to their web CommonJS bundle:

firebase-js-sdk/packages/firebase/package.json

Line 44 in 5b87b59

"browser": "dist/index.cjs.js",

The APF v10 doc doesn't mention the browser: field at all. This message is coming from ng build, but it could be emitted by Webpack...

I'll try to get an Angular CLI expert to weigh in on this soon...

[Splaktar]

OK, I got word from the Angular CLI team that:

The problem is that their browser version of the bundle is pointing to a cjs version.
The browser field is preferred over the module field.
https://github.com/angular/angular-cli/blob/8763c64768635297f4eacfcb095ddf71a359c130/packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/browser.ts#L69

So indeed, my second post appears to have identified the issue. Is it possible to switch the browser field of the Firebase JS packages to point to the .esm.js rather than the .cjs.js file?

This shouldn't break NodeJS server-side applications at all.

[akauppi]

@hsubox76 The recommendations on packaging node modules for both ESM and CommonJS/UMD use are here. I know some of Firebase team are looking into those. My hope is that the packaging will follow them, at some point.

I would also appreciate it if changes to packaging would not be done with Angular or any other single client in mind, as this issue implies, but the node.js standard. Then, all clients would be pleased.

[Splaktar]

@akauppi based on those NodeJS recommendations, it appears fine for the browsers field to point to the ESMs. As that field is ignored by NodeJS. I haven't seen any recommendation where pointing the browsers field to the CJS files is recommended.

It also suggests that the module field point to the ESMs for NodeJS support. But that already appears to be the case. Was there some other NodeJS-specific change that you want made to the packaging? Though this is a bit off-topic for a thread specific to Angular.

[rbogdy]

any news on a fix for this problem?

[hsubox76]

This PR in progress will hopefully address the issue: #3932

[jamesdaniels]

Fixed with Firebase JS SDK v8. AngularFire will support in 6.0.4 which will be released today.

[rbogdy]

I still get the "\app.module.ts depends on '@angular/fire/firestore'. CommonJS or AMD dependencies can cause optimization bailouts." warning even after updating AngularFire to 6.0.4

[alexis-mrc]

Same for me

[jamesdaniels]

This appears to be a false positive with ngcc, I don't see any indications that anything is built wrong despite the warning. It seems NG 11 isn't warning either, so I think it's a bug on the ngcc side angular/angularfire#2565 (comment)