Skip to content

Commit b254605

Browse files
sam-gcavolkovi
sam-gc
authored and
avolkovi
committed
Address (some) of the tree shaking issues w/ ReCaptcha (#3277)
* Address (some) of the recaptcha tree-shaking issues * Formatting
1 parent 17f8c28 commit b254605

File tree

5 files changed

+39
-36
lines changed

5 files changed

+39
-36
lines changed

‎packages-exp/auth-exp/src/core/user/additional_user_info.test.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,14 +181,21 @@ describe('core/user/additional_user_info', () => {
181181
});
182182

183183
it('for missing provider IDs in response but not in token', () => {
184-
const idToken = 'algorithm.' + base64Encode(JSON.stringify({'firebase': {'sign_in_provider': 'facebook.com'}})) + '.signature';
184+
const idToken =
185+
'algorithm.' +
186+
base64Encode(
187+
JSON.stringify({
188+
'firebase': { 'sign_in_provider': 'facebook.com' }
189+
})
190+
) +
191+
'.signature';
185192
const {
186193
isNewUser,
187194
providerId,
188195
username,
189196
profile
190197
} = _fromIdTokenResponse(
191-
idTokenResponse({ rawUserInfo: rawUserInfoWithLogin , idToken})
198+
idTokenResponse({ rawUserInfo: rawUserInfoWithLogin, idToken })
192199
)!;
193200
expect(isNewUser).to.be.false;
194201
expect(providerId).to.eq(ProviderId.FACEBOOK);

‎packages-exp/auth-exp/src/platform_browser/recaptcha/recaptcha_loader.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import { AUTH_WINDOW } from '../auth_window';
2929
import * as jsHelpers from '../load_js';
3030
import {
3131
_JSLOAD_CALLBACK,
32-
MOCK_RECAPTCHA_LOADER,
32+
MockReCaptchaLoaderImpl,
3333
ReCaptchaLoader,
3434
ReCaptchaLoaderImpl
3535
} from './recaptcha_loader';
@@ -52,7 +52,7 @@ describe('platform-browser/recaptcha/recaptcha_loader', () => {
5252

5353
describe('MockLoader', () => {
5454
it('returns a MockRecaptcha instance', async () => {
55-
expect(await MOCK_RECAPTCHA_LOADER.load(auth)).to.be.instanceOf(
55+
expect(await new MockReCaptchaLoaderImpl().load(auth)).to.be.instanceOf(
5656
MockReCaptcha
5757
);
5858
});

‎packages-exp/auth-exp/src/platform_browser/recaptcha/recaptcha_loader.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,13 +123,10 @@ export class ReCaptchaLoaderImpl implements ReCaptchaLoader {
123123
}
124124
}
125125

126-
class MockReCaptchaLoaderImpl implements ReCaptchaLoader {
126+
export class MockReCaptchaLoaderImpl implements ReCaptchaLoader {
127127
async load(auth: Auth): Promise<Recaptcha> {
128128
return new MockReCaptcha(auth);
129129
}
130130

131131
clearedOneInstance(): void {}
132132
}
133-
134-
export const MOCK_RECAPTCHA_LOADER: ReCaptchaLoader = new MockReCaptchaLoaderImpl();
135-
export const RECAPTCHA_LOADER: ReCaptchaLoader = new ReCaptchaLoaderImpl();

‎packages-exp/auth-exp/src/platform_browser/recaptcha/recaptcha_verifier.test.ts

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import { Endpoint } from '../../api';
2929
import { Auth } from '../../model/auth';
3030
import { AUTH_WINDOW } from '../auth_window';
3131
import { Parameters, Recaptcha } from './recaptcha';
32-
import { _JSLOAD_CALLBACK, MOCK_RECAPTCHA_LOADER } from './recaptcha_loader';
32+
import { _JSLOAD_CALLBACK, ReCaptchaLoader } from './recaptcha_loader';
3333
import { MockReCaptcha } from './recaptcha_mock';
3434
import { RecaptchaVerifier } from './recaptcha_verifier';
3535

@@ -41,6 +41,7 @@ describe('platform_browser/recaptcha/recaptcha_verifier.ts', () => {
4141
let container: HTMLElement;
4242
let verifier: RecaptchaVerifier;
4343
let parameters: Parameters;
44+
let recaptchaLoader: ReCaptchaLoader;
4445

4546
beforeEach(async () => {
4647
fetch.setUp();
@@ -54,6 +55,7 @@ describe('platform_browser/recaptcha/recaptcha_verifier.ts', () => {
5455
mockEndpoint(Endpoint.GET_RECAPTCHA_PARAM, {
5556
recaptchaSiteKey: 'recaptcha-key'
5657
});
58+
recaptchaLoader = verifier._recaptchaLoader;
5759
});
5860

5961
afterEach(() => {
@@ -64,7 +66,7 @@ describe('platform_browser/recaptcha/recaptcha_verifier.ts', () => {
6466
context('#render', () => {
6567
it('caches the promise if not completed and returns if called multiple times', () => {
6668
// This will force the loader to never return so the render promise never completes
67-
sinon.stub(MOCK_RECAPTCHA_LOADER, 'load').returns(new Promise(() => {}));
69+
sinon.stub(recaptchaLoader, 'load').returns(new Promise(() => {}));
6870
const renderPromise = verifier.render();
6971
expect(verifier.render()).to.eq(renderPromise);
7072
});
@@ -81,17 +83,15 @@ describe('platform_browser/recaptcha/recaptcha_verifier.ts', () => {
8183
});
8284

8385
it('sets loads the recaptcha per the app language code', async () => {
84-
sinon.spy(MOCK_RECAPTCHA_LOADER, 'load');
86+
sinon.spy(recaptchaLoader, 'load');
8587
await verifier.render();
86-
expect(MOCK_RECAPTCHA_LOADER.load).to.have.been.calledWith(auth, 'fr');
88+
expect(recaptchaLoader.load).to.have.been.calledWith(auth, 'fr');
8789
});
8890

8991
it('calls render on the underlying recaptcha widget', async () => {
9092
const widget = new MockReCaptcha(auth);
9193
sinon.spy(widget, 'render');
92-
sinon
93-
.stub(MOCK_RECAPTCHA_LOADER, 'load')
94-
.returns(Promise.resolve(widget));
94+
sinon.stub(recaptchaLoader, 'load').returns(Promise.resolve(widget));
9595
await verifier.render();
9696
expect(widget.render).to.have.been.calledWith(
9797
container.children[0],
@@ -100,7 +100,7 @@ describe('platform_browser/recaptcha/recaptcha_verifier.ts', () => {
100100
});
101101

102102
it('in case of error, resets render promise', async () => {
103-
sinon.stub(MOCK_RECAPTCHA_LOADER, 'load').returns(Promise.reject('nope'));
103+
sinon.stub(recaptchaLoader, 'load').returns(Promise.reject('nope'));
104104
const promise = verifier.render();
105105
await expect(promise).to.be.rejectedWith('nope');
106106
expect(verifier.render()).not.to.eq(promise);
@@ -111,9 +111,7 @@ describe('platform_browser/recaptcha/recaptcha_verifier.ts', () => {
111111
let recaptcha: Recaptcha;
112112
beforeEach(() => {
113113
recaptcha = new MockReCaptcha(auth);
114-
sinon
115-
.stub(MOCK_RECAPTCHA_LOADER, 'load')
116-
.returns(Promise.resolve(recaptcha));
114+
sinon.stub(recaptchaLoader, 'load').returns(Promise.resolve(recaptcha));
117115
});
118116

119117
it('returns immediately if response is available', async () => {
@@ -161,9 +159,7 @@ describe('platform_browser/recaptcha/recaptcha_verifier.ts', () => {
161159
context('#reset', () => {
162160
it('calls reset on the underlying widget', async () => {
163161
const recaptcha = new MockReCaptcha(auth);
164-
sinon
165-
.stub(MOCK_RECAPTCHA_LOADER, 'load')
166-
.returns(Promise.resolve(recaptcha));
162+
sinon.stub(recaptchaLoader, 'load').returns(Promise.resolve(recaptcha));
167163
sinon.spy(recaptcha, 'reset');
168164
await verifier.render();
169165
verifier._reset();

‎packages-exp/auth-exp/src/platform_browser/recaptcha/recaptcha_verifier.ts

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,15 @@ import { initializeAuth } from '../../core/auth/auth_impl';
2222
import { AuthErrorCode } from '../../core/errors';
2323
import { assert } from '../../core/util/assert';
2424
import { _isHttpOrHttps } from '../../core/util/location';
25+
import { ApplicationVerifier } from '../../model/application_verifier';
2526
import { Auth } from '../../model/auth';
2627
import { AUTH_WINDOW } from '../auth_window';
2728
import { Parameters, Recaptcha } from './recaptcha';
2829
import {
29-
MOCK_RECAPTCHA_LOADER,
30-
RECAPTCHA_LOADER,
31-
ReCaptchaLoader
30+
MockReCaptchaLoaderImpl,
31+
ReCaptchaLoader,
32+
ReCaptchaLoaderImpl
3233
} from './recaptcha_loader';
33-
import { ApplicationVerifier } from '../../model/application_verifier';
3434

3535
const DEFAULT_PARAMS: Parameters = {
3636
theme: 'light',
@@ -53,7 +53,7 @@ export class RecaptchaVerifier
5353
private readonly tokenChangeListeners = new Set<TokenCallback>();
5454
private renderPromise: Promise<number> | null = null;
5555

56-
private readonly recaptchaLoader: ReCaptchaLoader;
56+
readonly _recaptchaLoader: ReCaptchaLoader;
5757
private recaptcha: Recaptcha | null = null;
5858

5959
constructor(
@@ -75,9 +75,9 @@ export class RecaptchaVerifier
7575
this.container = container;
7676
this.parameters.callback = this.makeTokenCallback(this.parameters.callback);
7777

78-
this.recaptchaLoader = this.auth.settings.appVerificationDisabledForTesting
79-
? MOCK_RECAPTCHA_LOADER
80-
: RECAPTCHA_LOADER;
78+
this._recaptchaLoader = this.auth.settings.appVerificationDisabledForTesting
79+
? new MockReCaptchaLoaderImpl()
80+
: new ReCaptchaLoaderImpl();
8181

8282
this.validateStartingState();
8383
// TODO: Figure out if sdk version is needed
@@ -86,7 +86,7 @@ export class RecaptchaVerifier
8686
async verify(): Promise<string> {
8787
this.assertNotDestroyed();
8888
const id = await this.render();
89-
const recaptcha = this.assertedRecaptcha;
89+
const recaptcha = this.getAssertedRecaptcha();
9090

9191
const response = recaptcha.getResponse(id);
9292
if (response) {
@@ -134,14 +134,14 @@ export class RecaptchaVerifier
134134
_reset(): void {
135135
this.assertNotDestroyed();
136136
if (this.widgetId !== null) {
137-
this.assertedRecaptcha.reset(this.widgetId);
137+
this.getAssertedRecaptcha().reset(this.widgetId);
138138
}
139139
}
140140

141141
clear(): void {
142142
this.assertNotDestroyed();
143143
this.destroyed = true;
144-
this.recaptchaLoader.clearedOneInstance();
144+
this._recaptchaLoader.clearedOneInstance();
145145
if (!this.isInvisible) {
146146
this.container.childNodes.forEach(node => {
147147
this.container.removeChild(node);
@@ -193,7 +193,10 @@ export class RecaptchaVerifier
193193
container = guaranteedEmpty;
194194
}
195195

196-
this.widgetId = this.assertedRecaptcha.render(container, this.parameters);
196+
this.widgetId = this.getAssertedRecaptcha().render(
197+
container,
198+
this.parameters
199+
);
197200
}
198201

199202
return this.widgetId;
@@ -203,7 +206,7 @@ export class RecaptchaVerifier
203206
assert(_isHttpOrHttps() && !isWorker(), this.appName);
204207

205208
await domReady();
206-
this.recaptcha = await this.recaptchaLoader.load(
209+
this.recaptcha = await this._recaptchaLoader.load(
207210
this.auth,
208211
this.auth.languageCode || undefined
209212
);
@@ -213,7 +216,7 @@ export class RecaptchaVerifier
213216
this.parameters.sitekey = siteKey;
214217
}
215218

216-
private get assertedRecaptcha(): Recaptcha {
219+
private getAssertedRecaptcha(): Recaptcha {
217220
assert(this.recaptcha, this.appName);
218221
return this.recaptcha;
219222
}

0 commit comments

Comments
 (0)