PR feedback, fix compat tests

Author: sam-gc

packages-exp/auth-compat-exp/src/platform.ts

@@ -162,8 +162,13 @@ export function _getClientPlatform(): impl.ClientPlatform {
   return impl.ClientPlatform.BROWSER;
 }
 
+/** Quick check that indicates the platform *may* be Cordova */
+export function _isLikelyCordova(): boolean {
+  return _isAndroidOrIosCordovaScheme() && typeof document !== 'undefined';
+}
+
 export async function _isCordova(): Promise<boolean> {
-  if (!_isAndroidOrIosCordovaScheme() || typeof document === 'undefined') {
+  if (!_isLikelyCordova()) {
     return false;
   }
 

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

@@ -126,11 +126,31 @@ describe('popup_redirect/CompatPopupRedirectResolver', () => {
       );
     });
   });
+
+  context('_shouldInitProactively', () => {
+    it('returns true if platform may be cordova', () => {
+      sinon.stub(platform, '_isLikelyCordova').returns(true);
+      expect(compatResolver._shouldInitProactively).to.be.true;
+    });
+
+    it('returns true if cordova is false but browser value is true', () => {
+      sinon.stub(exp._getInstance<exp.PopupRedirectResolverInternal>(exp.browserPopupRedirectResolver), '_shouldInitProactively').value(true);
+      sinon.stub(platform, '_isLikelyCordova').returns(false);
+      expect(compatResolver._shouldInitProactively).to.be.true;
+    });
+
+    it('returns false if not cordova and not browser early init', () => {
+      sinon.stub(exp._getInstance<exp.PopupRedirectResolverInternal>(exp.browserPopupRedirectResolver), '_shouldInitProactively').value(false);
+      sinon.stub(platform, '_isLikelyCordova').returns(false);
+      expect(compatResolver._shouldInitProactively).to.be.false;
+    });
+  });
 });
 
 class FakeResolver implements exp.PopupRedirectResolverInternal {
   _completeRedirectFn = async (): Promise<null> => null;
   _redirectPersistence = exp.inMemoryPersistence;
+  _shouldInitProactively = true;
 
   _initialize(): Promise<exp.EventManager> {
     throw new Error('Method not implemented.');

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

@@ -16,9 +16,11 @@
  */
 
 import * as exp from '@firebase/auth-exp/internal';
-import { _isCordova } from './platform';
+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
@@ -40,11 +42,7 @@ 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 = exp._getInstance(
-      isCordova
-        ? exp.cordovaPopupRedirectResolver
-        : exp.browserPopupRedirectResolver
-    );
+    this.underlyingResolver = isCordova ? CORDOVA_RESOLVER : BROWSER_RESOLVER;
     return this.assertedUnderlyingResolver._initialize(auth);
   }
 
@@ -87,6 +85,10 @@ export class CompatPopupRedirectResolver
     return this.assertedUnderlyingResolver._originValidation(auth);
   }
 
+  get _shouldInitProactively(): boolean {
+    return _isLikelyCordova() || BROWSER_RESOLVER._shouldInitProactively;
+  }
+
   private get assertedUnderlyingResolver(): exp.PopupRedirectResolverInternal {
     _assert(this.underlyingResolver, exp.AuthErrorCode.INTERNAL_ERROR);
     return this.underlyingResolver;

packages-exp/auth-exp/test/helpers/redirect_persistence.ts

@@ -26,8 +26,10 @@ export class RedirectPersistence extends InMemoryPersistence {
   async _get<T extends PersistenceValue>(key: string): Promise<T | null> {
     if (key.includes('pendingRedirect')) {
       return this.hasPendingRedirect.toString() as T;
+    } else if (key.includes('redirectUser')) {
+      return this.redirectUser as T | null;
     }
 
-    return this.redirectUser as T | null;
+    throw new Error(`Unexpected redirect persistence key requested: ${key}`);
   }
 }