Merge remote-tracking branch 'origin/master' into TestingHooksSpi

Author: dconeybe

.changeset/mean-candles-approve.md

@@ -0,0 +1,5 @@
+---
+'@firebase/auth': patch
+---
+
+Fix auth event uncancellable bug #7383

config/webpack.test.js

@@ -17,6 +17,7 @@
 
 const path = require('path');
 const webpack = require('webpack');
+const NodePolyfillPlugin = require('node-polyfill-webpack-plugin');
 
 /**
  * A regular expression used to replace Firestore's and Storage's platform-
@@ -28,6 +29,11 @@ const PLATFORM_RE = /^(.*)\/platform\/([^.\/]*)(\.ts)?$/;
 module.exports = {
   mode: 'development',
   devtool: 'source-map',
+  optimization: {
+    runtimeChunk: false,
+    splitChunks: false,
+    minimize: false
+  },
   module: {
     rules: [
       {
@@ -98,7 +104,7 @@ module.exports = {
     modules: ['node_modules', path.resolve(__dirname, '../../node_modules')],
     mainFields: ['browser', 'module', 'main'],
     extensions: ['.js', '.ts'],
-    symlinks: false
+    symlinks: true
   },
   plugins: [
     new webpack.NormalModuleReplacementPlugin(PLATFORM_RE, resource => {
@@ -108,6 +114,7 @@ module.exports = {
         `$1/platform/${targetPlatform}/$2.ts`
       );
     }),
+    new NodePolyfillPlugin(),
     new webpack.EnvironmentPlugin([
       'RTDB_EMULATOR_PORT',
       'RTDB_EMULATOR_NAMESPACE'

integration/firebase/test/namespace.test.ts

@@ -15,7 +15,11 @@
  * limitations under the License.
  */
 
-import firebase from 'firebase/compat';
+import firebase from 'firebase/compat/app';
+import 'firebase/compat/auth';
+import 'firebase/compat/database';
+import 'firebase/compat/storage';
+import 'firebase/compat/messaging';
 import * as namespaceDefinition from './namespaceDefinition.json';
 import validateNamespace from './validator';
 

integration/firestore/package.json

@@ -31,7 +31,7 @@
     "mocha": "9.2.2",
     "ts-loader": "8.4.0",
     "typescript": "4.2.2",
-    "webpack": "4.46.0",
+    "webpack": "5.76.0",
     "webpack-stream": "6.1.2"
   }
 }

package.json

@@ -45,7 +45,7 @@
     "size-report": "ts-node-script scripts/size_report/report_binary_size.ts",
     "modular-export-size-report": "ts-node-script scripts/size_report/report_modular_export_binary_size.ts",
     "api-report": "lerna run --scope @firebase/* api-report",
-    "postinstall": "yarn --cwd repo-scripts/changelog-generator build",
+    "postinstall": "patch-package && yarn --cwd repo-scripts/changelog-generator build",
     "sa": "ts-node-script repo-scripts/size-analysis/cli.ts",
     "api-documenter-devsite": "ts-node-script repo-scripts/api-documenter/src/start.ts",
     "format": "ts-node ./scripts/format/format.ts"
@@ -122,7 +122,7 @@
     "karma-sourcemap-loader": "0.4.0",
     "karma-spec-reporter": "0.0.34",
     "karma-summary-reporter": "3.1.1",
-    "karma-webpack": "4.0.2",
+    "karma-webpack": "5.0.0",
     "lcov-result-merger": "3.1.0",
     "lerna": "4.0.0",
     "listr": "0.14.3",
@@ -132,9 +132,12 @@
     "mkdirp": "1.0.4",
     "mocha": "9.2.2",
     "mz": "2.7.0",
+    "node-polyfill-webpack-plugin": "2.0.1",
     "npm-run-all": "4.1.5",
     "nyc": "15.1.0",
     "ora": "5.4.1",
+    "patch-package": "7.0.0",
+    "postinstall-postinstall": "2.1.0",
     "prettier": "2.8.7",
     "protractor": "5.4.2",
     "request": "2.88.2",
@@ -151,7 +154,7 @@
     "typedoc": "0.16.11",
     "typescript": "4.7.4",
     "watch": "1.0.2",
-    "webpack": "4.46.0",
+    "webpack": "5.76.0",
     "yargs": "17.7.1"
   }
 }

packages/auth-compat/test/helpers/helpers.ts

@@ -17,6 +17,7 @@
 
 import * as sinon from 'sinon';
 import firebase from '@firebase/app-compat';
+import '@firebase/auth-compat';
 import { Provider } from '@firebase/component';
 import '../..';
 

packages/auth/src/core/auth/auth_impl.ts

@@ -638,18 +638,37 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
         ? nextOrObserver
         : nextOrObserver.next.bind(nextOrObserver);
 
+    let isUnsubscribed = false;
+
     const promise = this._isInitialized
       ? Promise.resolve()
       : this._initializationPromise;
     _assert(promise, this, AuthErrorCode.INTERNAL_ERROR);
     // The callback needs to be called asynchronously per the spec.
     // eslint-disable-next-line @typescript-eslint/no-floating-promises
-    promise.then(() => cb(this.currentUser));
+    promise.then(() => {
+      if (isUnsubscribed) {
+        return;
+      }
+      cb(this.currentUser);
+    });
 
     if (typeof nextOrObserver === 'function') {
-      return subscription.addObserver(nextOrObserver, error, completed);
+      const unsubscribe = subscription.addObserver(
+        nextOrObserver,
+        error,
+        completed
+      );
+      return () => {
+        isUnsubscribed = true;
+        unsubscribe();
+      };
     } else {
-      return subscription.addObserver(nextOrObserver);
+      const unsubscribe = subscription.addObserver(nextOrObserver);
+      return () => {
+        isUnsubscribed = true;
+        unsubscribe();
+      };
     }
   }
 

packages/auth/test/helpers/integration/helpers.ts

@@ -17,7 +17,7 @@
 
 import * as sinon from 'sinon';
 import { deleteApp, initializeApp } from '@firebase/app';
-import { Auth, User } from '../../../src/model/public_types';
+import { Auth, User } from '@firebase/auth';
 
 import { getAuth, connectAuthEmulator } from '../../../'; // Use browser OR node dist entrypoint depending on test env.
 import { _generateEventId } from '../../../src/core/util/event_id';

packages/firestore/test/integration/api/database.test.ts

@@ -798,9 +798,11 @@ apiDescribe('Database', persistence => {
         .then(snap => {
           expect(snap.exists()).to.be.true;
           expect(snap.data()).to.deep.equal({ a: 1 });
-          expect(snap.metadata.hasPendingWrites).to.be.false;
-        })
-        .then(() => storeEvent.assertNoAdditionalEvents());
+          // This event could be a metadata change for fromCache as well.
+          // We comment this line out to reduce flakiness.
+          // TODO(b/295872012): Figure out a way to check for all scenarios.
+          // expect(snap.metadata.hasPendingWrites).to.be.false;
+        });
     });
   });
 
@@ -827,9 +829,11 @@ apiDescribe('Database', persistence => {
         .then(() => storeEvent.awaitEvent())
         .then(snap => {
           expect(snap.data()).to.deep.equal(changedData);
-          expect(snap.metadata.hasPendingWrites).to.be.false;
-        })
-        .then(() => storeEvent.assertNoAdditionalEvents());
+          // This event could be a metadata change for fromCache as well.
+          // We comment this line out to reduce flakiness.
+          // TODO(b/295872012): Figure out a way to check for all scenarios.
+          // expect(snap.metadata.hasPendingWrites).to.be.false;
+        });
     });
   });
 

packages/firestore/test/integration/api/numeric_transforms.test.ts

@@ -102,7 +102,10 @@ apiDescribe('Numeric Transforms:', persistence => {
     });
   });
 
-  it('increment existing integer with integer', async () => {
+  // TODO(b/295872012): This test is skipped due to a timeout test flakiness
+  // We should investigate if this is an acutal bug.
+  // eslint-disable-next-line no-restricted-properties
+  it.skip('increment existing integer with integer', async () => {
     await withTestSetup(async () => {
       await writeInitialData({ sum: 1337 });
       await updateDoc(docRef, 'sum', increment(1));
@@ -158,7 +161,11 @@ apiDescribe('Numeric Transforms:', persistence => {
     });
   });
 
-  it('multiple double increments', async () => {
+  // TODO(b/295872012): This test is skipped due to test flakiness:
+  // AssertionError: expected 0.122 to be close to 0.111 +/- 0.000001
+  // We should investigate the root cause, it might be an acutal bug.
+  // eslint-disable-next-line no-restricted-properties
+  it.skip('multiple double increments', async () => {
     await withTestSetup(async () => {
       await writeInitialData({ sum: 0.0 });
 

packages/firestore/test/integration/api/query.test.ts

@@ -444,7 +444,11 @@ apiDescribe('Queries', persistence => {
     });
   });
 
-  it('can listen for the same query with different options', () => {
+  // TODO(b/295872012): This test is skipped due to the flakiness around the
+  // checks of hasPendingWrites.
+  // We should investigate if this is an acutal bug.
+  // eslint-disable-next-line no-restricted-properties
+  it.skip('can listen for the same query with different options', () => {
     const testDocs = { a: { v: 'a' }, b: { v: 'b' } };
     return withTestCollection(persistence, testDocs, coll => {
       const storeEvent = new EventsAccumulator<QuerySnapshot>();

patches/karma-webpack+5.0.0.patch

@@ -0,0 +1,23 @@
+diff --git a/node_modules/karma-webpack/lib/webpack/plugin.js b/node_modules/karma-webpack/lib/webpack/plugin.js
+index 47b993c..3b75a9e 100644
+--- a/node_modules/karma-webpack/lib/webpack/plugin.js
++++ b/node_modules/karma-webpack/lib/webpack/plugin.js
+@@ -1,4 +1,5 @@
+ const fs = require('fs');
++const path = require('path');
+ 
+ class KW_WebpackPlugin {
+   constructor(options) {
+@@ -14,9 +15,10 @@ class KW_WebpackPlugin {
+       // read generated file content and store for karma preprocessor
+       this.controller.bundlesContent = {};
+       stats.toJson().assets.forEach((webpackFileObj) => {
+-        const filePath = `${compiler.options.output.path}/${
++        const filePath = path.resolve(
++          compiler.options.output.path,
+           webpackFileObj.name
+-        }`;
++        );
+         this.controller.bundlesContent[webpackFileObj.name] = fs.readFileSync(
+           filePath,
+           'utf-8'

repo-scripts/size-analysis/package.json

@@ -26,7 +26,7 @@
     "rollup-plugin-replace": "2.2.0",
     "rollup-plugin-typescript2": "0.31.2",
     "@rollup/plugin-virtual": "2.1.0",
-    "webpack": "4.46.0",
+    "webpack": "5.76.0",
     "@types/webpack": "5.28.0",
     "webpack-virtual-modules": "0.5.0",
     "child-process-promise": "2.2.1",