Make initializeApp idempotent (#5207)

Feiyang1 · web-flow · commit a3cbe719b1bd · 2021-07-28T13:56:14.000-07:00
* Add deepEqual function

* make initializeApp idempotent

* make initializeApp in app-compat idempotent

* Create fair-readers-drum.md

* address comments
diff --git a/.changeset/fair-readers-drum.md b/.changeset/fair-readers-drum.md
@@ -0,0 +1,5 @@
+---
+"@firebase/util": minor
+---
+
+Added deepEqual for comparing objects
diff --git a/packages-exp/app-compat/src/firebaseNamespaceCore.ts b/packages-exp/app-compat/src/firebaseNamespaceCore.ts
@@ -109,6 +109,8 @@ export function createFirebaseNamespaceCore(
 
   /**
    * Create a new App instance (name must be unique).
+   *
+   * This function is idempotent. It can be called more than once and return the same instance using the same options and config.
    */
   function initializeAppCompat(
     options: FirebaseOptions,
@@ -118,6 +120,11 @@ export function createFirebaseNamespaceCore(
       options,
       rawConfig
     ) as _FirebaseAppExp;
+
+    if (contains(apps, app.name)) {
+      return apps[app.name];
+    }
+
     const appCompat = new firebaseAppImpl(app, namespace);
     apps[app.name] = appCompat;
     return appCompat;
diff --git a/packages-exp/app-compat/test/firebaseAppCompat.test.ts b/packages-exp/app-compat/test/firebaseAppCompat.test.ts
@@ -346,15 +346,61 @@ function firebaseAppTests(
       expect(firebase.apps.length).to.eq(2);
     });
 
-    it('duplicate DEFAULT initialize is an error.', () => {
-      firebase.initializeApp({});
-      expect(() => firebase.initializeApp({})).throws(/\[DEFAULT\].*exists/i);
+    it('initializeApp can be called more than once and returns the same instance if the options and config are the same', () => {
+      const app = firebase.initializeApp(
+        {
+          apiKey: 'test1'
+        },
+        { automaticDataCollectionEnabled: true }
+      );
+      expect(
+        firebase.initializeApp(
+          {
+            apiKey: 'test1'
+          },
+          { automaticDataCollectionEnabled: true }
+        )
+      ).to.equal(app);
+    });
+
+    it('duplicate DEFAULT initialize with different options is an error.', () => {
+      firebase.initializeApp({ apiKey: 'key1' });
+      expect(() => firebase.initializeApp({ apiKey: 'key2' })).throws(
+        /\[DEFAULT\].*exists/i
+      );
+    });
+
+    it('duplicate named App initialize with different options is an error.', () => {
+      firebase.initializeApp({ apiKey: 'key1', appId: 'id' }, 'abc');
+      expect(() => firebase.initializeApp({ apiKey: 'key1' }, 'abc')).throws(
+        /'abc'.*exists/i
+      );
     });
 
-    it('duplicate named App initialize is an error.', () => {
-      firebase.initializeApp({}, 'abc');
+    it('duplicate DEFAULT initialize with different config is an error.', () => {
+      firebase.initializeApp(
+        { apiKey: 'key1' },
+        { automaticDataCollectionEnabled: true }
+      );
+      expect(() =>
+        firebase.initializeApp(
+          { apiKey: 'key1' },
+          { automaticDataCollectionEnabled: false }
+        )
+      ).throws(/\[DEFAULT\].*exists/i);
+    });
 
-      expect(() => firebase.initializeApp({}, 'abc')).throws(/'abc'.*exists/i);
+    it('duplicate named App initialize with different config is an error.', () => {
+      firebase.initializeApp(
+        { apiKey: 'key1' },
+        { name: 'abc', automaticDataCollectionEnabled: true }
+      );
+      expect(() =>
+        firebase.initializeApp(
+          { apiKey: 'key1' },
+          { name: 'abc', automaticDataCollectionEnabled: false }
+        )
+      ).throws(/'abc'.*exists/i);
     });
 
     it('automaticDataCollectionEnabled is `false` by default', () => {
diff --git a/packages-exp/app-exp/src/api.test.ts b/packages-exp/app-exp/src/api.test.ts
@@ -74,15 +74,85 @@ describe('API tests', () => {
       expect(app2.name).to.equal(appName);
     });
 
-    it('throws when creating duplicate DEDAULT Apps', () => {
-      initializeApp({});
-      expect(() => initializeApp({})).throws(/\[DEFAULT\].*exists/i);
+    it('can be called more than once and returns the same instance if the options and config are the same', () => {
+      const app = initializeApp(
+        {
+          apiKey: 'test1'
+        },
+        { automaticDataCollectionEnabled: true }
+      );
+      expect(
+        initializeApp(
+          {
+            apiKey: 'test1'
+          },
+          { automaticDataCollectionEnabled: true }
+        )
+      ).to.equal(app);
+    });
+
+    it('throws when creating duplicate DEDAULT Apps with different options', () => {
+      initializeApp({
+        apiKey: 'test1'
+      });
+      expect(() =>
+        initializeApp({
+          apiKey: 'test2'
+        })
+      ).throws(/\[DEFAULT\].*exists/i);
+    });
+
+    it('throws when creating duplicate named Apps with different options', () => {
+      const appName = 'MyApp';
+      initializeApp(
+        {
+          apiKey: 'test1'
+        },
+        appName
+      );
+      expect(() =>
+        initializeApp(
+          {
+            apiKey: 'test2'
+          },
+          appName
+        )
+      ).throws(/'MyApp'.*exists/i);
+    });
+
+    it('throws when creating duplicate DEDAULT Apps with different config values', () => {
+      initializeApp(
+        {
+          apiKey: 'test1'
+        },
+        { automaticDataCollectionEnabled: true }
+      );
+      expect(() =>
+        initializeApp(
+          {
+            apiKey: 'test1'
+          },
+          { automaticDataCollectionEnabled: false }
+        )
+      ).throws(/\[DEFAULT\].*exists/i);
     });
 
-    it('throws when creating duplicate named Apps', () => {
+    it('throws when creating duplicate named Apps with different config values', () => {
       const appName = 'MyApp';
-      initializeApp({}, appName);
-      expect(() => initializeApp({}, appName)).throws(/'MyApp'.*exists/i);
+      initializeApp(
+        {
+          apiKey: 'test1'
+        },
+        { name: appName, automaticDataCollectionEnabled: true }
+      );
+      expect(() =>
+        initializeApp(
+          {
+            apiKey: 'test1'
+          },
+          { name: appName, automaticDataCollectionEnabled: false }
+        )
+      ).throws(/'MyApp'.*exists/i);
     });
 
     it('takes an object as the second parameter to create named App', () => {
diff --git a/packages-exp/app-exp/src/api.ts b/packages-exp/app-exp/src/api.ts
@@ -39,6 +39,7 @@ import {
   LogOptions,
   setUserLogHandler
 } from '@firebase/logger';
+import { deepEqual } from '@firebase/util';
 
 /**
  * The current SDK version.
@@ -129,8 +130,17 @@ export function initializeApp(
     });
   }
 
-  if (_apps.has(name)) {
-    throw ERROR_FACTORY.create(AppError.DUPLICATE_APP, { appName: name });
+  const existingApp = _apps.get(name) as FirebaseAppImpl;
+  if (existingApp) {
+    // return the existing app if options and config deep equal the ones in the existing app.
+    if (
+      deepEqual(options, existingApp.options) &&
+      deepEqual(config, existingApp.config)
+    ) {
+      return existingApp;
+    } else {
+      throw ERROR_FACTORY.create(AppError.DUPLICATE_APP, { appName: name });
+    }
   }
 
   const container = new ComponentContainer(name);
diff --git a/packages-exp/app-exp/src/errors.ts b/packages-exp/app-exp/src/errors.ts
@@ -31,7 +31,8 @@ const ERRORS: ErrorMap<AppError> = {
     "No Firebase App '{$appName}' has been created - " +
     'call Firebase App.initializeApp()',
   [AppError.BAD_APP_NAME]: "Illegal App name: '{$appName}",
-  [AppError.DUPLICATE_APP]: "Firebase App named '{$appName}' already exists",
+  [AppError.DUPLICATE_APP]:
+    "Firebase App named '{$appName}' already exists with different options or config",
   [AppError.APP_DELETED]: "Firebase App named '{$appName}' already deleted",
   [AppError.INVALID_APP_ARGUMENT]:
     'firebase.{$appName}() takes either no argument or a ' +
diff --git a/packages-exp/app-exp/src/firebaseApp.ts b/packages-exp/app-exp/src/firebaseApp.ts
@@ -30,6 +30,13 @@ import { ERROR_FACTORY, AppError } from './errors';
 export class FirebaseAppImpl implements FirebaseApp {
   private readonly _options: FirebaseOptions;
   private readonly _name: string;
+  /**
+   * Original config values passed in as a constructor parameter.
+   * It is only used to compare with another config object to support idempotent initializeApp().
+   *
+   * Updating automaticDataCollectionEnabled on the App instance will not change its value in _config.
+   */
+  private readonly _config: Required<FirebaseAppSettings>;
   private _automaticDataCollectionEnabled: boolean;
   private _isDeleted = false;
   private readonly _container: ComponentContainer;
@@ -40,6 +47,7 @@ export class FirebaseAppImpl implements FirebaseApp {
     container: ComponentContainer
   ) {
     this._options = { ...options };
+    this._config = { ...config };
     this._name = config.name;
     this._automaticDataCollectionEnabled =
       config.automaticDataCollectionEnabled;
@@ -69,6 +77,11 @@ export class FirebaseAppImpl implements FirebaseApp {
     return this._options;
   }
 
+  get config(): Required<FirebaseAppSettings> {
+    this.checkDestroyed();
+    return this._config;
+  }
+
   get container(): ComponentContainer {
     return this._container;
   }
diff --git a/packages/util/src/obj.ts b/packages/util/src/obj.ts
@@ -52,3 +52,41 @@ export function map<K extends string, V, U>(
   }
   return res as { [key in K]: U };
 }
+
+/**
+ * Deep equal two objects. Support Arrays and Objects.
+ */
+export function deepEqual(a: object, b: object): boolean {
+  if (a === b) {
+    return true;
+  }
+
+  const aKeys = Object.keys(a);
+  const bKeys = Object.keys(b);
+  for (const k of aKeys) {
+    if (!bKeys.includes(k)) {
+      return false;
+    }
+
+    const aProp = (a as Record<string, unknown>)[k];
+    const bProp = (b as Record<string, unknown>)[k];
+    if (isObject(aProp) && isObject(bProp)) {
+      if (!deepEqual(aProp, bProp)) {
+        return false;
+      }
+    } else if (aProp !== bProp) {
+      return false;
+    }
+  }
+
+  for (const k of bKeys) {
+    if (!aKeys.includes(k)) {
+      return false;
+    }
+  }
+  return true;
+}
+
+function isObject(thing: unknown): thing is object {
+  return thing !== null && typeof thing === 'object';
+}
diff --git a/packages/util/test/object.test.ts b/packages/util/test/object.test.ts
