Merge branch 'master' into fis-modularization

Author: ChaoqunCHEN

.changeset/small-grapes-teach.md

@@ -0,0 +1,5 @@
+---
+"@firebase/component": patch
+---
+
+Correctly delete services created by modular SDKs when calling provider.delete()

package.json

@@ -143,7 +143,7 @@
     "sinon": "9.0.3",
     "sinon-chai": "3.5.0",
     "source-map-loader": "1.0.2",
-    "terser": "4.8.0",
+    "terser": "5.2.1",
     "ts-loader": "8.0.3",
     "ts-node": "8.10.2",
     "tslint": "6.1.3",

packages-exp/app-exp/src/api.test.ts

@@ -29,7 +29,10 @@ import {
   onLog
 } from './api';
 import { DEFAULT_ENTRY_NAME } from './constants';
-import { _FirebaseAppInternal } from '@firebase/app-types-exp';
+import {
+  _FirebaseAppInternal,
+  _FirebaseService
+} from '@firebase/app-types-exp';
 import {
   _clearComponents,
   _components,
@@ -175,6 +178,33 @@ describe('API tests', () => {
       deleteApp(app).catch(() => {});
       expect(getApps().length).to.equal(0);
     });
+
+    it('waits for all services being deleted', async () => {
+      _clearComponents();
+      let count = 0;
+      const comp1 = new Component(
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        'test1' as any,
+        _container =>
+          ({
+            _delete: async () => {
+              await Promise.resolve();
+              expect(count).to.equal(0);
+              count++;
+            }
+          } as _FirebaseService),
+        ComponentType.PUBLIC
+      );
+      _registerComponent(comp1);
+
+      const app = initializeApp({});
+      // create service instance
+      const test1Provider = _getProvider(app, 'test1' as any);
+      test1Provider.getImmediate();
+
+      await deleteApp(app);
+      expect(count).to.equal(1);
+    });
   });
 
   describe('registerVersion', () => {

packages-exp/app-exp/test/util.ts

@@ -25,7 +25,7 @@ export class TestService implements _FirebaseService {
     return this.app_;
   }
 
-  delete(): Promise<void> {
+  _delete(): Promise<void> {
     return new Promise((resolve: (v?: void) => void) => {
       setTimeout(() => resolve(), 10);
     });

packages-exp/app-types-exp/index.d.ts

@@ -113,7 +113,7 @@ export interface _FirebaseService {
    * Delete the service and free it's resources - called from
    * {@link @firebase/app-exp#deleteApp | deleteApp()}
    */
-  delete(): Promise<void>;
+  _delete(): Promise<void>;
 }
 
 export interface VersionService {

packages-exp/firebase-exp/package.json

@@ -45,7 +45,7 @@
     "rollup-plugin-license": "2.2.0",
     "rollup-plugin-node-resolve": "5.2.0",
     "rollup-plugin-sourcemaps": "0.6.2",
-    "rollup-plugin-terser": "6.1.0",
+    "rollup-plugin-terser": "7.0.0",
     "rollup-plugin-typescript2": "0.27.2",
     "rollup-plugin-uglify": "6.0.4",
     "gulp": "4.0.2",

packages-exp/functions-exp/src/service.ts

@@ -95,7 +95,7 @@ export class FunctionsService implements _FirebaseService {
     });
   }
 
-  delete(): Promise<void> {
+  _delete(): Promise<void> {
     return this.deleteService();
   }
 

packages/component/src/provider.test.ts

@@ -19,6 +19,7 @@ import { expect } from 'chai';
 import { fake, SinonSpy } from 'sinon';
 import { ComponentContainer } from './component_container';
 import { FirebaseService } from '@firebase/app-types/private';
+import { _FirebaseService } from '@firebase/app-types-exp';
 import { Provider } from './provider';
 import { getFakeApp, getFakeComponent } from '../test/util';
 import '../test/setup';
@@ -202,7 +203,7 @@ describe('Provider', () => {
     });
 
     describe('delete()', () => {
-      it('calls delete() on the service instance that implements FirebaseService', () => {
+      it('calls delete() on the service instance that implements legacy FirebaseService', () => {
         const deleteFake = fake();
         const myService: FirebaseService = {
           app: getFakeApp(),
@@ -226,6 +227,29 @@ describe('Provider', () => {
 
         expect(deleteFake).to.have.been.called;
       });
+
+      it('calls delete() on the service instance that implements next FirebaseService', () => {
+        const deleteFake = fake();
+        const myService: _FirebaseService = {
+          app: getFakeApp(),
+          _delete: deleteFake
+        };
+
+        // provide factory and create a service instance
+        provider.setComponent(
+          getFakeComponent(
+            'test',
+            () => myService,
+            false,
+            InstantiationMode.EAGER
+          )
+        );
+
+        // eslint-disable-next-line @typescript-eslint/no-floating-promises
+        provider.delete();
+
+        expect(deleteFake).to.have.been.called;
+      });
     });
 
     describe('clearCache()', () => {

packages/component/src/provider.ts

@@ -170,12 +170,16 @@ export class Provider<T extends Name> {
   async delete(): Promise<void> {
     const services = Array.from(this.instances.values());
 
-    await Promise.all(
-      services
-        .filter(service => 'INTERNAL' in service)
+    await Promise.all([
+      ...services
+        .filter(service => 'INTERNAL' in service) // legacy services
         // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        .map(service => (service as any).INTERNAL!.delete())
-    );
+        .map(service => (service as any).INTERNAL!.delete()),
+      ...services
+        .filter(service => '_delete' in service) // modularized services
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        .map(service => (service as any)._delete())
+    ]);
   }
 
   isComponentSet(): boolean {

packages/firebase/package.json

@@ -66,7 +66,7 @@
     "rollup-plugin-license": "2.2.0",
     "rollup-plugin-node-resolve": "5.2.0",
     "rollup-plugin-sourcemaps": "0.6.2",
-    "rollup-plugin-terser": "6.1.0",
+    "rollup-plugin-terser": "7.0.0",
     "rollup-plugin-typescript2": "0.27.2",
     "rollup-plugin-uglify": "6.0.4",
     "typescript": "4.0.2"

packages/firestore/exp/src/api/components.ts

@@ -23,7 +23,7 @@ import {
   OfflineComponentProvider,
   OnlineComponentProvider
 } from '../../../src/core/component_provider';
-import {handleUserChange, LocalStore} from '../../../src/local/local_store';
+import { handleUserChange, LocalStore } from '../../../src/local/local_store';
 import { Deferred } from '../../../src/util/promise';
 import { logDebug } from '../../../src/util/log';
 import { SyncEngine } from '../../../src/core/sync_engine';
@@ -71,7 +71,7 @@ export async function setOfflineComponentProvider(
   // When a user calls clearPersistence() in one client, all other clients
   // need to be terminated to allow the delete to succeed.
   offlineComponentProvider.persistence.setDatabaseDeletedListener(() =>
-    firestore.delete()
+    firestore._delete()
   );
   offlineDeferred.resolve(offlineComponentProvider);
 }

packages/firestore/exp/src/api/database.ts

@@ -69,7 +69,8 @@ const LOG_TAG = 'Firestore';
  * The root reference to the Firestore database and the entry point for the
  * tree-shakeable SDK.
  */
-export class Firestore extends LiteFirestore
+export class Firestore
+  extends LiteFirestore
   implements firestore.FirebaseFirestore, _FirebaseService {
   readonly _queue = new AsyncQueue();
   readonly _persistenceKey: string;
@@ -320,7 +321,7 @@ export function terminate(
 ): Promise<void> {
   _removeServiceInstance(firestore.app, 'firestore-exp');
   const firestoreImpl = cast(firestore, Firestore);
-  return firestoreImpl.delete();
+  return firestoreImpl._delete();
 }
 
 function verifyNotInitialized(firestore: Firestore): void {

packages/firestore/lite/src/api/database.ts

@@ -95,7 +95,7 @@ export class Firestore
     return new DatabaseId(app.options.projectId!);
   }
 
-  delete(): Promise<void> {
+  _delete(): Promise<void> {
     if (!this._terminateTask) {
       this._terminateTask = this._terminate();
     }
@@ -117,7 +117,7 @@ export class Firestore
   // TODO(firestoreexp): `deleteApp()` should call the delete method above,
   // but it still calls INTERNAL.delete().
   INTERNAL = {
-    delete: () => this.delete()
+    delete: () => this._delete()
   };
 }
 
@@ -142,5 +142,5 @@ export function terminate(
 ): Promise<void> {
   _removeServiceInstance(firestore.app, 'firestore/lite');
   const firestoreClient = cast(firestore, Firestore);
-  return firestoreClient.delete();
+  return firestoreClient._delete();
 }

packages/firestore/package.json

@@ -86,7 +86,7 @@
     "rollup-plugin-node-resolve": "5.2.0",
     "rollup-plugin-replace": "2.2.0",
     "rollup-plugin-sourcemaps": "0.6.2",
-    "rollup-plugin-terser": "6.1.0",
+    "rollup-plugin-terser": "7.0.0",
     "rollup-plugin-typescript2": "0.27.2",
     "ts-node": "8.10.2",
     "typescript": "4.0.2"

repo-scripts/size-analysis/analysis-helper.ts

@@ -116,20 +116,20 @@ export async function extractDependenciesAndSize(
     externalDepsNotResolvedOutput,
     'utf-8'
   );
-  const externalDepsResolvedOutputContentMinimized: terser.MinifyOutput = terser.minify(
+  const externalDepsResolvedOutputContentMinimized = await terser.minify(
     externalDepsResolvedOutputContent,
     {
-      output: {
+      format: {
         comments: false
       },
       mangle: { toplevel: true },
       compress: false
     }
   );
-  const externalDepsNotResolvedOutputContentMinimized: terser.MinifyOutput = terser.minify(
+  const externalDepsNotResolvedOutputContentMinimized = await terser.minify(
     externalDepsNotResolvedOutputContent,
     {
-      output: {
+      format: {
         comments: false
       },
       mangle: { toplevel: true },

repo-scripts/size-analysis/package.json

@@ -26,7 +26,7 @@
     "rollup-plugin-typescript2": "0.27.2",
     "tmp": "0.2.1",
     "typescript": "4.0.2",
-    "terser": "4.8.0",
+    "terser": "5.2.1",
     "yargs": "15.4.1",
     "@firebase/util": "0.3.1"
   },

scripts/size_report/report_binary_size.ts

@@ -65,7 +65,7 @@ function generateReportForCDNScripts(): Report[] {
 }
 
 // NPM packages
-function generateReportForNPMPackages(): Report[] {
+async function generateReportForNPMPackages(): Promise<Report[]> {
   const reports: Report[] = [];
   const fields = [
     'main',
@@ -81,41 +81,50 @@ function generateReportForNPMPackages(): Report[] {
     execSync('npx lerna ls --json --scope @firebase/*').toString()
   );
 
+  const taskPromises: Promise<void>[] = [];
   for (const pkg of packageInfo) {
     // we traverse the dir in order to include binaries for submodules, e.g. @firebase/firestore/memory
     // Currently we only traverse 1 level deep because we don't have any submodule deeper than that.
     traverseDirs(pkg.location, collectBinarySize, 0, 1);
   }
 
-  function collectBinarySize(path: string) {
-    const packageJsonPath = `${path}/package.json`;
-    if (!fs.existsSync(packageJsonPath)) {
-      return;
-    }
-
-    const packageJson = require(packageJsonPath);
+  await Promise.all(taskPromises);
 
-    for (const field of fields) {
-      if (packageJson[field]) {
-        const filePath = `${path}/${packageJson[field]}`;
-        const rawCode = fs.readFileSync(filePath, 'utf-8');
+  return reports;
 
-        // remove comments and whitespaces, then get size
-        const { code } = terser.minify(rawCode, {
-          output: {
-            comments: false
-          },
-          mangle: false,
-          compress: false
-        });
+  function collectBinarySize(path: string) {
+    const promise = new Promise<void>(async resolve => {
+      const packageJsonPath = `${path}/package.json`;
+      if (!fs.existsSync(packageJsonPath)) {
+        return;
+      }
 
-        const size = Buffer.byteLength(code!, 'utf-8');
-        reports.push(makeReportObject(packageJson.name, field, size));
+      const packageJson = require(packageJsonPath);
+
+      for (const field of fields) {
+        if (packageJson[field]) {
+          const filePath = `${path}/${packageJson[field]}`;
+          const rawCode = fs.readFileSync(filePath, 'utf-8');
+
+          // remove comments and whitespaces, then get size
+          const { code } = await terser.minify(rawCode, {
+            format: {
+              comments: false
+            },
+            mangle: false,
+            compress: false
+          });
+
+          const size = Buffer.byteLength(code!, 'utf-8');
+          reports.push(makeReportObject(packageJson.name, field, size));
+        }
       }
-    }
-  }
 
-  return reports;
+      resolve();
+    });
+
+    taskPromises.push(promise);
+  }
 }
 
 function traverseDirs(
@@ -147,10 +156,10 @@ function makeReportObject(sdk: string, type: string, value: number): Report {
   };
 }
 
-function generateSizeReport(): BinarySizeRequestBody {
+async function generateSizeReport(): Promise<BinarySizeRequestBody> {
   const reports: Report[] = [
     ...generateReportForCDNScripts(),
-    ...generateReportForNPMPackages()
+    ...(await generateReportForNPMPackages())
   ];
 
   for (const r of reports) {
@@ -168,5 +177,6 @@ function generateSizeReport(): BinarySizeRequestBody {
   };
 }
 
-const report = generateSizeReport();
-upload(report, RequestEndpoint.BINARY_SIZE);
+generateSizeReport().then(report => {
+  upload(report, RequestEndpoint.BINARY_SIZE);
+});