Make node and browser entry points identical (#5318)

sam-gc · Feiyang1 · web-flow · commit 0a91a906baec · 2021-08-18T16:51:16.000-07:00
* Overhaul node/browser entry points so that they are identical; use rollup-plugin-alias to overwrite browser-specific features

* Formatting, license

* Revert bad change

* Update packages-exp/auth-exp/rollup.config.shared.js

Co-authored-by: Feiyang &lt;feiyangc@google.com&gt;

* Fix popup/redirect stuff

* Fix popup/redirect stuff

Co-authored-by: Feiyang &lt;feiyangc@google.com&gt;
diff --git a/packages-exp/auth-compat-exp/src/popup_redirect.test.ts b/packages-exp/auth-compat-exp/src/popup_redirect.test.ts
@@ -26,6 +26,15 @@ import { FirebaseApp } from '@firebase/app-compat';
 use(sinonChai);
 
 describe('popup_redirect/CompatPopupRedirectResolver', () => {
+  // Do not run these tests in node; in node, this resolver
+  // is never instantiated.
+  if (typeof window === 'undefined') {
+    console.log(
+      'Skipping popup/redirect resolver tests in non-browser environment'
+    );
+    return;
+  }
+
   let compatResolver: CompatPopupRedirectResolver;
   let auth: exp.AuthImpl;
 
@@ -75,9 +84,11 @@ describe('popup_redirect/CompatPopupRedirectResolver', () => {
 
     beforeEach(() => {
       underlyingResolver = sinon.createStubInstance(FakeResolver);
-      ((compatResolver as unknown) as {
-        underlyingResolver: exp.PopupRedirectResolverInternal;
-      }).underlyingResolver = underlyingResolver;
+      (
+        compatResolver as unknown as {
+          underlyingResolver: exp.PopupRedirectResolverInternal;
+        }
+      ).underlyingResolver = underlyingResolver;
       provider = new exp.GoogleAuthProvider();
     });
 
diff --git a/packages-exp/auth-compat-exp/src/popup_redirect.ts b/packages-exp/auth-compat-exp/src/popup_redirect.ts
@@ -19,16 +19,17 @@ import * as exp from '@firebase/auth-exp/internal';
 import { _isCordova, _isLikelyCordova } from './platform';
 
 const _assert: typeof exp._assert = exp._assert;
-const BROWSER_RESOLVER: exp.PopupRedirectResolverInternal = exp._getInstance(
-  exp.browserPopupRedirectResolver
-);
-const CORDOVA_RESOLVER: exp.PopupRedirectResolverInternal = exp._getInstance(
-  exp.cordovaPopupRedirectResolver
-);
 
 /** Platform-agnostic popup-redirect resolver */
 export class CompatPopupRedirectResolver
-  implements exp.PopupRedirectResolverInternal {
+  implements exp.PopupRedirectResolverInternal
+{
+  // Create both resolvers for dynamic resolution later
+  private readonly browserResolver: exp.PopupRedirectResolverInternal =
+    exp._getInstance(exp.browserPopupRedirectResolver);
+  private readonly cordovaResolver: exp.PopupRedirectResolverInternal =
+    exp._getInstance(exp.cordovaPopupRedirectResolver);
+  // The actual resolver in use: either browserResolver or cordovaResolver.
   private underlyingResolver: exp.PopupRedirectResolverInternal | null = null;
   _redirectPersistence = exp.browserSessionPersistence;
 
@@ -85,7 +86,7 @@ export class CompatPopupRedirectResolver
   }
 
   get _shouldInitProactively(): boolean {
-    return _isLikelyCordova() || BROWSER_RESOLVER._shouldInitProactively;
+    return _isLikelyCordova() || this.browserResolver._shouldInitProactively;
   }
 
   private get assertedUnderlyingResolver(): exp.PopupRedirectResolverInternal {
@@ -101,6 +102,8 @@ export class CompatPopupRedirectResolver
     // We haven't yet determined whether or not we're in Cordova; go ahead
     // and determine that state now.
     const isCordova = await _isCordova();
-    this.underlyingResolver = isCordova ? CORDOVA_RESOLVER : BROWSER_RESOLVER;
+    this.underlyingResolver = isCordova
+      ? this.cordovaResolver
+      : this.browserResolver;
   }
 }
diff --git a/packages-exp/auth-exp/index.cordova.ts b/packages-exp/auth-exp/index.cordova.ts
@@ -30,8 +30,7 @@ import { initializeAuth } from './src';
 import { registerAuth } from './src/core/auth/register';
 import { ClientPlatform } from './src/core/util/version';
 
-// Core functionality shared by all clients
-export * from './src';
+export * from './index.shared';
 
 // Cordova also supports indexedDB / browserSession / browserLocal
 export { indexedDBLocalPersistence } from './src/platform_browser/persistence/indexed_db';
diff --git a/packages-exp/auth-exp/index.node.ts b/packages-exp/auth-exp/index.node.ts
@@ -1,6 +1,6 @@
 /**
  * @license
- * Copyright 2017 Google LLC
+ * Copyright 2021 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,48 +15,5 @@
  * limitations under the License.
  */
 
-/**
- * This is the file that people using Node.js will actually import. You should
- * only include this file if you have something specific about your
- * implementation that mandates having a separate entrypoint. Otherwise you can
- * just use index.browser.ts
- */
-
-import * as fetchImpl from 'node-fetch';
-
-import { FirebaseApp, getApp, _getProvider } from '@firebase/app-exp';
-import { Auth } from './src/model/public_types';
-
-import { initializeAuth } from './src';
-import { registerAuth } from './src/core/auth/register';
-import { FetchProvider } from './src/core/util/fetch_provider';
-import { ClientPlatform } from './src/core/util/version';
-
-// Initialize the fetch polyfill, the types are slightly off so just cast and hope for the best
-FetchProvider.initialize(
-  fetchImpl.default as unknown as typeof fetch,
-  fetchImpl.Headers as unknown as typeof Headers,
-  fetchImpl.Response as unknown as typeof Response
-);
-
-// Core functionality shared by all clients
-export * from './src';
-export {
-  FactorId,
-  ProviderId,
-  SignInMethod,
-  OperationType,
-  ActionCodeOperation
-} from './src/model/enum_maps';
-
-export function getAuth(app: FirebaseApp = getApp()): Auth {
-  const provider = _getProvider(app, 'auth-exp');
-
-  if (provider.isInitialized()) {
-    return provider.getImmediate();
-  }
-
-  return initializeAuth(app);
-}
-
-registerAuth(ClientPlatform.NODE);
+// Node entry point is identical to browser entry point
+export * from './index';
diff --git a/packages-exp/auth-exp/index.rn.ts b/packages-exp/auth-exp/index.rn.ts
@@ -33,7 +33,7 @@ import { ClientPlatform } from './src/core/util/version';
 import { getReactNativePersistence } from './src/platform_react_native/persistence/react_native';
 
 // Core functionality shared by all clients
-export * from './src';
+export * from './index.shared';
 
 /**
  * An implementation of {@link Persistence} of type 'LOCAL' for use in React
diff --git a/packages-exp/auth-exp/index.shared.ts b/packages-exp/auth-exp/index.shared.ts
@@ -0,0 +1,31 @@
+/**
+ * @license
+ * Copyright 2021 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.
+ */
+
+// Public types
+export * from './src/model/public_types';
+
+// Helper maps (not used internally)
+export {
+  FactorId,
+  ProviderId,
+  SignInMethod,
+  OperationType,
+  ActionCodeOperation
+} from './src/model/enum_maps';
+
+// Core functionality shared by all clients
+export * from './src';
diff --git a/packages-exp/auth-exp/index.ts b/packages-exp/auth-exp/index.ts
@@ -21,17 +21,6 @@
  * limitations under the License.
  */
 
-import { FirebaseApp, getApp, _getProvider } from '@firebase/app-exp';
-
-import { initializeAuth } from './src';
-import { registerAuth } from './src/core/auth/register';
-import { ClientPlatform } from './src/core/util/version';
-import { browserLocalPersistence } from './src/platform_browser/persistence/local_storage';
-import { browserSessionPersistence } from './src/platform_browser/persistence/session_storage';
-import { indexedDBLocalPersistence } from './src/platform_browser/persistence/indexed_db';
-import { browserPopupRedirectResolver } from './src/platform_browser/popup_redirect';
-import { Auth } from './src/model/public_types';
-
 // Public types
 export * from './src/model/public_types';
 
@@ -47,64 +36,65 @@ export {
 // Core functionality shared by all clients
 export * from './src';
 
-// Additional DOM dependend functionality
+// Additional DOM dependend functionality; we need to import and then
+// export separately so that the rollup alias will work (for aliasing these
+// imports in node environments to no-ops and errors... see
+// src/platform_node/index.ts).
 
 // persistence
-export { browserLocalPersistence } from './src/platform_browser/persistence/local_storage';
-export { browserSessionPersistence } from './src/platform_browser/persistence/session_storage';
-export { indexedDBLocalPersistence } from './src/platform_browser/persistence/indexed_db';
+import { browserLocalPersistence } from './src/platform_browser/persistence/local_storage';
+import { browserSessionPersistence } from './src/platform_browser/persistence/session_storage';
+import { indexedDBLocalPersistence } from './src/platform_browser/persistence/indexed_db';
 
 // providers
-export { PhoneAuthProvider } from './src/platform_browser/providers/phone';
+import { PhoneAuthProvider } from './src/platform_browser/providers/phone';
 
 // strategies
-export {
+import {
   signInWithPhoneNumber,
   linkWithPhoneNumber,
   reauthenticateWithPhoneNumber,
   updatePhoneNumber
 } from './src/platform_browser/strategies/phone';
-export {
+import {
   signInWithPopup,
   linkWithPopup,
   reauthenticateWithPopup
 } from './src/platform_browser/strategies/popup';
-export {
+import {
   signInWithRedirect,
   linkWithRedirect,
   reauthenticateWithRedirect,
   getRedirectResult
 } from './src/platform_browser/strategies/redirect';
 
-export { RecaptchaVerifier } from './src/platform_browser/recaptcha/recaptcha_verifier';
-export { browserPopupRedirectResolver } from './src/platform_browser/popup_redirect';
+import { RecaptchaVerifier } from './src/platform_browser/recaptcha/recaptcha_verifier';
+import { browserPopupRedirectResolver } from './src/platform_browser/popup_redirect';
 
 // MFA
-export { PhoneMultiFactorGenerator } from './src/platform_browser/mfa/assertions/phone';
-
-/**
- * Returns the Auth instance associated with the provided {@link @firebase/app#FirebaseApp}.
- * If no instance exists, initializes an Auth instance with platform-specific default dependencies.
- *
- * @param app - The Firebase App.
- *
- * @public
- */
-export function getAuth(app: FirebaseApp = getApp()): Auth {
-  const provider = _getProvider(app, 'auth-exp');
+import { PhoneMultiFactorGenerator } from './src/platform_browser/mfa/assertions/phone';
 
-  if (provider.isInitialized()) {
-    return provider.getImmediate();
-  }
+// Initialization and registration of Auth
+import { getAuth } from './src/platform_browser';
 
-  return initializeAuth(app, {
-    popupRedirectResolver: browserPopupRedirectResolver,
-    persistence: [
-      indexedDBLocalPersistence,
-      browserLocalPersistence,
-      browserSessionPersistence
-    ]
-  });
-}
-
-registerAuth(ClientPlatform.BROWSER);
+export {
+  browserLocalPersistence,
+  browserSessionPersistence,
+  indexedDBLocalPersistence,
+  PhoneAuthProvider,
+  signInWithPhoneNumber,
+  linkWithPhoneNumber,
+  reauthenticateWithPhoneNumber,
+  updatePhoneNumber,
+  signInWithPopup,
+  linkWithPopup,
+  reauthenticateWithPopup,
+  signInWithRedirect,
+  linkWithRedirect,
+  reauthenticateWithRedirect,
+  getRedirectResult,
+  RecaptchaVerifier,
+  browserPopupRedirectResolver,
+  PhoneMultiFactorGenerator,
+  getAuth
+};
diff --git a/packages-exp/auth-exp/index.webworker.ts b/packages-exp/auth-exp/index.webworker.ts
@@ -27,7 +27,7 @@ import { ClientPlatform } from './src/core/util/version';
 import { indexedDBLocalPersistence } from './src/platform_browser/persistence/indexed_db';
 
 // Core functionality shared by all clients
-export * from './src';
+export * from './index.shared';
 
 // persistence
 export { indexedDBLocalPersistence } from './src/platform_browser/persistence/indexed_db';
diff --git a/packages-exp/auth-exp/package.json b/packages-exp/auth-exp/package.json
@@ -38,10 +38,10 @@
     "test:cordova": "karma start --single-run --cordova",
     "test:cordova:debug": "karma start --auto-watch --cordova",
     "test:node": "run-s test:node:unit test:node:integration:local",
-    "test:node:unit": "node ./scripts/run-node-tests.js",
-    "test:node:integration": "node ./scripts/run-node-tests.js --integration",
-    "test:node:integration:local": "node ./scripts/run-node-tests.js --integration --local",
-    "test:webdriver": "rollup -c test/integration/webdriver/static/rollup.config.js && node ./scripts/run-node-tests.js --webdriver",
+    "test:node:unit": "ts-node -O '{\"module\": \"commonjs\", \"target\": \"es6\"}' scripts/run_node_tests.ts",
+    "test:node:integration": "ts-node -O '{\"module\": \"commonjs\", \"target\": \"es6\"}' scripts/run_node_tests.ts --integration",
+    "test:node:integration:local": "ts-node -O '{\"module\": \"commonjs\", \"target\": \"es6\"}' scripts/run_node_tests.ts --integration --local",
+    "test:webdriver": "rollup -c test/integration/webdriver/static/rollup.config.js && ts-node -O '{\"module\": \"commonjs\", \"target\": \"es6\"}' scripts/run_node_tests.ts --webdriver",
     "api-report": "api-extractor run --local --verbose && ts-node-script ../../repo-scripts/prune-dts/prune-dts.ts --input dist/auth-exp-public.d.ts --output dist/auth-exp-public.d.ts",
     "predoc": "node ../../scripts/exp/remove-exp.js temp",
     "doc": "api-documenter markdown --input temp --output docs",
@@ -85,4 +85,4 @@
     "reportDir": "./coverage/node"
   },
   "esm5": "dist/esm5/index.js"
-}
+}
diff --git a/packages-exp/auth-exp/rollup.config.shared.js b/packages-exp/auth-exp/rollup.config.shared.js
@@ -18,6 +18,7 @@
 import strip from '@rollup/plugin-strip';
 import typescriptPlugin from 'rollup-plugin-typescript2';
 import json from '@rollup/plugin-json';
+import alias from '@rollup/plugin-alias';
 import typescript from 'typescript';
 import pkg from './package.json';
 import { importPathTransformer } from '../../scripts/exp/ts-transform-import-path';
@@ -37,6 +38,21 @@ const commonPlugins = [
   })
 ];
 
+/**
+ * Node has the same entry point as browser, but browser-specific exports
+ * are turned into either no-ops or errors. See src/platform_node/index.ts for
+ * more info. This regex tests explicitly ./src/platform_browser so that the
+ * only impacted file is the main index.ts
+ */
+const nodeAliasPlugin = alias({
+  entries: [
+    {
+      find: /^\.\/src\/platform_browser(\/.*)?$/,
+      replacement: `./src/platform_node`
+    }
+  ]
+});
+
 export function getConfig({ isReleaseBuild }) {
   /**
    * ES5 Builds
@@ -90,7 +106,7 @@ export function getConfig({ isReleaseBuild }) {
         internal: 'internal/index.ts'
       },
       output: [{ dir: 'dist/node', format: 'cjs', sourcemap: true }],
-      plugins: es5BuildPlugins,
+      plugins: [nodeAliasPlugin, ...es5BuildPlugins],
       external: id => deps.some(dep => id === dep || id.startsWith(`${dep}/`))
     },
     /**
diff --git a/packages-exp/auth-exp/scripts/run-node-tests.js b/packages-exp/auth-exp/scripts/run-node-tests.js
diff --git a/packages-exp/auth-exp/scripts/run_node_tests.ts b/packages-exp/auth-exp/scripts/run_node_tests.ts
diff --git a/packages-exp/auth-exp/src/core/errors.ts b/packages-exp/auth-exp/src/core/errors.ts
diff --git a/packages-exp/auth-exp/src/platform_browser/index.ts b/packages-exp/auth-exp/src/platform_browser/index.ts
diff --git a/packages-exp/auth-exp/src/platform_node/index.ts b/packages-exp/auth-exp/src/platform_node/index.ts
