PR feedback

Author: sam-gc

packages-exp/auth-exp/index.cordova.ts

@@ -16,7 +16,7 @@
  */
 
 /**
- * This is the file that people using React Native will actually import. You
+ * This is the file that people using Cordova 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.ts

packages-exp/auth-exp/src/platform_cordova/popup_redirect.test.ts

@@ -34,13 +34,13 @@ describe('platform_cordova/popup_redirect', () => {
 
   beforeEach(async () => {
     auth = await testAuth();
+    attachExpectedPlugins();
     resolver = new (cordovaPopupRedirectResolver as SingletonInstantiator<PopupRedirectResolver>)();
   });
 
   describe('_openRedirect plugin checks', () => {
     // TODO: Rest of the tests go here
     it('does not reject if all plugins installed', () => {
-      setExpectedPluginsButMissing([]);
       expect(() =>
         resolver._openRedirect(
           auth,
@@ -51,55 +51,58 @@ describe('platform_cordova/popup_redirect', () => {
     });
 
     it('rejects if universal links is missing', () => {
-      setExpectedPluginsButMissing(['universalLinks.subscribe']);
+      removeProp(window, 'universalLinks');
       expect(() =>
         resolver._openRedirect(
           auth,
           new GoogleAuthProvider(),
           AuthEventType.SIGN_IN_VIA_REDIRECT
         )
-      ).to.throw(FirebaseError, 'auth/invalid-cordova-configuration');
+      ).to.throw(FirebaseError, 'auth/invalid-cordova-configuration')
+      .that.has.deep.property('customData', {appName: 'test-app', missingPlugin: 'cordova-universal-links-plugin-fix'});
     });
 
     it('rejects if build info is missing', () => {
-      setExpectedPluginsButMissing(['BuildInfo.packageName']);
+      removeProp(window.BuildInfo, 'packageName');
       expect(() =>
         resolver._openRedirect(
           auth,
           new GoogleAuthProvider(),
           AuthEventType.SIGN_IN_VIA_REDIRECT
         )
-      ).to.throw(FirebaseError, 'auth/invalid-cordova-configuration');
+      ).to.throw(FirebaseError, 'auth/invalid-cordova-configuration')
+      .that.has.deep.property('customData', {appName: 'test-app', missingPlugin: 'cordova-plugin-buildInfo'});
     });
 
-    it('rejects if browsertab openUrl', () => {
-      setExpectedPluginsButMissing(['cordova.plugins.browsertab.openUrl']);
+    it('rejects if browsertab openUrl is missing', () => {
+      removeProp(window.cordova.plugins.browsertab, 'openUrl');
       expect(() =>
         resolver._openRedirect(
           auth,
           new GoogleAuthProvider(),
           AuthEventType.SIGN_IN_VIA_REDIRECT
         )
-      ).to.throw(FirebaseError, 'auth/invalid-cordova-configuration');
+      ).to.throw(FirebaseError, 'auth/invalid-cordova-configuration')
+      .that.has.deep.property('customData', {appName: 'test-app', missingPlugin: 'cordova-plugin-browsertab'});
     });
 
     it('rejects if InAppBrowser is missing', () => {
-      setExpectedPluginsButMissing(['cordova.InAppBrowser.open']);
+      removeProp(window.cordova.InAppBrowser, 'open');
       expect(() =>
         resolver._openRedirect(
           auth,
           new GoogleAuthProvider(),
           AuthEventType.SIGN_IN_VIA_REDIRECT
         )
-      ).to.throw(FirebaseError, 'auth/invalid-cordova-configuration');
+      ).to.throw(FirebaseError, 'auth/invalid-cordova-configuration')
+      .that.has.deep.property('customData', {appName: 'test-app', missingPlugin: 'cordova-plugin-inappbrowser'});
     });
   });
 });
 
-function setExpectedPluginsButMissing(missingPlugins: string[]): void {
-  // eslint-disable @typescript-eslint/no-any We don't want a strictly typed window
-  const win: any = window;
-  // Eventually these will be replaced with mocks
+function attachExpectedPlugins(): void {
+  // Eventually these will be replaced with full mocks
+  const win = window as unknown as Record<string, unknown>;
   win.cordova = {
     plugins: {
       browsertab: {
@@ -115,17 +118,10 @@ function setExpectedPluginsButMissing(missingPlugins: string[]): void {
     subscribe: () => {}
   };
   win.BuildInfo = {
-    packageName: 'com.name.package'
+    packageName: 'com.example.name.package'
   };
-
-  for (const missing of missingPlugins) {
-    const parts = missing.split('.');
-    const last = parts.pop()!;
-    // eslint-disable @typescript-eslint/no-any We're just traversing an object map
-    let obj: any = win;
-    for (const p of parts) {
-      obj = obj[p];
-    }
-    delete obj[last];
-  }
 }
+
+function removeProp(obj: unknown, prop: string):void {
+  delete (obj as Record<string, unknown>)[prop];
+}

packages-exp/auth-exp/src/platform_cordova/popup_redirect.ts

@@ -25,10 +25,13 @@ import {
   PopupRedirectResolver
 } from '../model/popup_redirect';
 import { AuthPopup } from '../platform_browser/util/popup';
-import { _fail } from '../core/util/assert';
+import { _assert, _fail } from '../core/util/assert';
 import { AuthErrorCode } from '../core/errors';
 
 class CordovaPopupRedirectResolver implements PopupRedirectResolver {
+  readonly _redirectPersistence = browserSessionPersistence;
+  _completeRedirectFn: () => Promise<null> = async () => null;
+
   _initialize(_auth: Auth): Promise<EventManager> {
     throw new Error('Method not implemented.');
   }
@@ -50,8 +53,6 @@ class CordovaPopupRedirectResolver implements PopupRedirectResolver {
   ): void {
     throw new Error('Method not implemented.');
   }
-  readonly _redirectPersistence = browserSessionPersistence;
-  _completeRedirectFn: () => Promise<null> = async () => null;
 }
 
 function checkCordovaConfiguration(auth: Auth): void {
@@ -60,36 +61,30 @@ function checkCordovaConfiguration(auth: Auth): void {
   // Note that cordova-universal-links-plugin has been abandoned.
   // A fork with latest fixes is available at:
   // https://www.npmjs.com/package/cordova-universal-links-plugin-fix
-  if (typeof window?.universalLinks?.subscribe !== 'function') {
-    _fail(auth, AuthErrorCode.INVALID_CORDOVA_CONFIGURATION, {
+  _assert(typeof window?.universalLinks?.subscribe === 'function', auth, AuthErrorCode.INVALID_CORDOVA_CONFIGURATION, {
       missingPlugin: 'cordova-universal-links-plugin-fix'
     });
-  }
 
   // https://www.npmjs.com/package/cordova-plugin-buildinfo
-  if (typeof window?.BuildInfo?.packageName === 'undefined') {
-    _fail(auth, AuthErrorCode.INVALID_CORDOVA_CONFIGURATION, {
+  _assert (typeof window?.BuildInfo?.packageName !== 'undefined', auth, AuthErrorCode.INVALID_CORDOVA_CONFIGURATION, {
       missingPlugin: 'cordova-plugin-buildInfo'
     });
-  }
 
   // https://github.com/google/cordova-plugin-browsertab
-  if (typeof window?.cordova?.plugins?.browsertab?.openUrl !== 'function') {
-    _fail(auth, AuthErrorCode.INVALID_CORDOVA_CONFIGURATION, {
+  _assert(typeof window?.cordova?.plugins?.browsertab?.openUrl === 'function',
+    auth, AuthErrorCode.INVALID_CORDOVA_CONFIGURATION, {
       missingPlugin: 'cordova-plugin-browsertab'
     });
-  }
 
   // https://cordova.apache.org/docs/en/latest/reference/cordova-plugin-inappbrowser/
-  if (typeof window?.cordova?.InAppBrowser?.open !== 'function') {
-    _fail(auth, AuthErrorCode.INVALID_CORDOVA_CONFIGURATION, {
+  _assert(typeof window?.cordova?.InAppBrowser?.open === 'function',
+    auth, AuthErrorCode.INVALID_CORDOVA_CONFIGURATION, {
       missingPlugin: 'cordova-plugin-inappbrowser'
     });
-  }
 }
 
 /**
- * An implementation of {@link @firebase/auth-types#PopupRedirectResolver} suitable for browser
+ * An implementation of {@link @firebase/auth-types#PopupRedirectResolver} suitable for Cordova
  * based applications.
  *
  * @public