Skip to content

Commit 2ec6473

Browse files
sam-gcsam-gc
committed
PR feedback
1 parent 9dc95f2 commit 2ec6473

File tree

5 files changed

+153
-182
lines changed

5 files changed

+153
-182
lines changed

‎packages-exp/auth-exp/src/core/strategies/abstract_popup_redirect_operation.test.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,7 @@ import { testAuth, testUser } from '../../../test/helpers/mock_auth';
3030
import { makeMockPopupRedirectResolver } from '../../../test/helpers/mock_popup_redirect_resolver';
3131
import { Auth } from '../../model/auth';
3232
import {
33-
AuthEvent,
34-
AuthEventType,
35-
EventManager,
36-
PopupRedirectResolver
33+
AuthEvent, AuthEventType, EventManager, PopupRedirectResolver
3734
} from '../../model/popup_redirect';
3835
import { AuthEventManager } from '../auth/auth_event_manager';
3936
import { AUTH_ERROR_FACTORY, AuthErrorCode } from '../errors';
@@ -86,12 +83,12 @@ describe('src/core/strategies/abstract_popup_redirect_operation', () => {
8683
);
8784
idpStubs._signIn.returns(
8885
Promise.resolve(
89-
new UserCredentialImpl(
90-
testUser(auth, 'uid'),
91-
ProviderId.GOOGLE,
92-
{ ...TEST_ID_TOKEN_RESPONSE },
93-
OperationType.SIGN_IN
94-
)
86+
new UserCredentialImpl({
87+
user: testUser(auth, 'uid'),
88+
providerId: ProviderId.GOOGLE,
89+
_tokenResponse: { ...TEST_ID_TOKEN_RESPONSE },
90+
operationType: OperationType.SIGN_IN
91+
})
9592
)
9693
);
9794
});

‎packages-exp/auth-exp/src/core/strategies/anonymous.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,11 @@ export async function signInAnonymously(
2929
const credential = AnonymousProvider.credential();
3030
if (auth.currentUser?.isAnonymous) {
3131
// If an anonymous user is already signed in, no need to sign them in again.
32-
return new UserCredentialImpl(
33-
auth.currentUser,
34-
null,
35-
undefined,
36-
externs.OperationType.SIGN_IN
37-
);
32+
return new UserCredentialImpl({
33+
user: auth.currentUser,
34+
providerId: null,
35+
operationType: externs.OperationType.SIGN_IN
36+
});
3837
}
3938
return signInWithCredential(auth, credential);
4039
}

‎packages-exp/auth-exp/src/core/strategies/popup.test.ts

Lines changed: 81 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,7 @@ import * as chaiAsPromised from 'chai-as-promised';
2020
import * as sinon from 'sinon';
2121
import * as sinonChai from 'sinon-chai';
2222

23-
import {
24-
OperationType,
25-
PopupRedirectResolver,
26-
ProviderId
27-
} from '@firebase/auth-types-exp';
23+
import { OperationType, PopupRedirectResolver, ProviderId } from '@firebase/auth-types-exp';
2824
import { FirebaseError } from '@firebase/util';
2925

3026
import { delay } from '../../../test/helpers/delay';
@@ -43,11 +39,8 @@ import * as eid from '../util/event_id';
4339
import { AuthPopup } from '../util/popup';
4440
import * as idpTasks from './idp';
4541
import {
46-
_AUTH_EVENT_TIMEOUT,
47-
_POLL_WINDOW_CLOSE_TIMEOUT,
48-
linkWithPopup,
49-
reauthenticateWithPopup,
50-
signInWithPopup
42+
_AUTH_EVENT_TIMEOUT, _POLL_WINDOW_CLOSE_TIMEOUT, linkWithPopup, reauthenticateWithPopup,
43+
signInWithPopup
5144
} from './popup';
5245

5346
use(sinonChai);
@@ -96,12 +89,11 @@ describe('src/core/strategies/popup', () => {
9689

9790
context('signInWithPopup', () => {
9891
it('completes the full flow', async () => {
99-
const cred = new UserCredentialImpl(
100-
testUser(auth, 'uid'),
101-
ProviderId.GOOGLE,
102-
undefined,
103-
OperationType.SIGN_IN
104-
);
92+
const cred = new UserCredentialImpl({
93+
user: testUser(auth, 'uid'),
94+
providerId: ProviderId.GOOGLE,
95+
operationType: OperationType.SIGN_IN
96+
});
10597
idpStubs._signIn.returns(Promise.resolve(cred));
10698
const promise = signInWithPopup(auth, provider, resolver);
10799
iframeEvent({
@@ -111,12 +103,11 @@ describe('src/core/strategies/popup', () => {
111103
});
112104

113105
it('ignores events for another event id', async () => {
114-
const cred = new UserCredentialImpl(
115-
testUser(auth, 'uid'),
116-
ProviderId.GOOGLE,
117-
undefined,
118-
OperationType.SIGN_IN
119-
);
106+
const cred = new UserCredentialImpl({
107+
user: testUser(auth, 'uid'),
108+
providerId: ProviderId.GOOGLE,
109+
operationType: OperationType.SIGN_IN
110+
});
120111
idpStubs._signIn.returns(Promise.resolve(cred));
121112
const promise = signInWithPopup(auth, provider, resolver);
122113
iframeEvent({
@@ -137,12 +128,11 @@ describe('src/core/strategies/popup', () => {
137128
});
138129

139130
it('does not call idp tasks if event is error', async () => {
140-
const cred = new UserCredentialImpl(
141-
testUser(auth, 'uid'),
142-
ProviderId.GOOGLE,
143-
undefined,
144-
OperationType.SIGN_IN
145-
);
131+
const cred = new UserCredentialImpl({
132+
user: testUser(auth, 'uid'),
133+
providerId: ProviderId.GOOGLE,
134+
operationType: OperationType.SIGN_IN
135+
});
146136
idpStubs._signIn.returns(Promise.resolve(cred));
147137
const promise = signInWithPopup(auth, provider, resolver);
148138
iframeEvent({
@@ -162,12 +152,11 @@ describe('src/core/strategies/popup', () => {
162152
});
163153

164154
it('does not error if the poll timeout trips', async () => {
165-
const cred = new UserCredentialImpl(
166-
testUser(auth, 'uid'),
167-
ProviderId.GOOGLE,
168-
undefined,
169-
OperationType.SIGN_IN
170-
);
155+
const cred = new UserCredentialImpl({
156+
user: testUser(auth, 'uid'),
157+
providerId: ProviderId.GOOGLE,
158+
operationType: OperationType.SIGN_IN
159+
});
171160
idpStubs._signIn.returns(Promise.resolve(cred));
172161
const promise = signInWithPopup(auth, provider, resolver);
173162
delay(() => {
@@ -181,12 +170,11 @@ describe('src/core/strategies/popup', () => {
181170
});
182171

183172
it('does error if the poll timeout and event timeout trip', async () => {
184-
const cred = new UserCredentialImpl(
185-
testUser(auth, 'uid'),
186-
ProviderId.GOOGLE,
187-
undefined,
188-
OperationType.SIGN_IN
189-
);
173+
const cred = new UserCredentialImpl({
174+
user: testUser(auth, 'uid'),
175+
providerId: ProviderId.GOOGLE,
176+
operationType: OperationType.SIGN_IN
177+
});
190178
idpStubs._signIn.returns(Promise.resolve(cred));
191179
const promise = signInWithPopup(auth, provider, resolver);
192180
delay(() => {
@@ -224,12 +212,11 @@ describe('src/core/strategies/popup', () => {
224212
});
225213

226214
it('cancels the task if called consecutively', async () => {
227-
const cred = new UserCredentialImpl(
228-
testUser(auth, 'uid'),
229-
ProviderId.GOOGLE,
230-
undefined,
231-
OperationType.SIGN_IN
232-
);
215+
const cred = new UserCredentialImpl({
216+
user: testUser(auth, 'uid'),
217+
providerId: ProviderId.GOOGLE,
218+
operationType: OperationType.SIGN_IN
219+
});
233220
idpStubs._signIn.returns(Promise.resolve(cred));
234221
const firstPromise = signInWithPopup(auth, provider, resolver);
235222
const secondPromise = signInWithPopup(auth, provider, resolver);
@@ -251,12 +238,11 @@ describe('src/core/strategies/popup', () => {
251238
});
252239

253240
it('completes the full flow', async () => {
254-
const cred = new UserCredentialImpl(
241+
const cred = new UserCredentialImpl({
255242
user,
256-
ProviderId.GOOGLE,
257-
undefined,
258-
OperationType.LINK
259-
);
243+
providerId: ProviderId.GOOGLE,
244+
operationType: OperationType.LINK
245+
});
260246
idpStubs._link.returns(Promise.resolve(cred));
261247
const promise = linkWithPopup(user, provider, resolver);
262248
iframeEvent({
@@ -266,12 +252,11 @@ describe('src/core/strategies/popup', () => {
266252
});
267253

268254
it('ignores events for another event id', async () => {
269-
const cred = new UserCredentialImpl(
255+
const cred = new UserCredentialImpl({
270256
user,
271-
ProviderId.GOOGLE,
272-
undefined,
273-
OperationType.LINK
274-
);
257+
providerId: ProviderId.GOOGLE,
258+
operationType: OperationType.LINK
259+
});
275260
idpStubs._link.returns(Promise.resolve(cred));
276261
const promise = linkWithPopup(user, provider, resolver);
277262
iframeEvent({
@@ -292,12 +277,11 @@ describe('src/core/strategies/popup', () => {
292277
});
293278

294279
it('does not call idp tasks if event is error', async () => {
295-
const cred = new UserCredentialImpl(
280+
const cred = new UserCredentialImpl({
296281
user,
297-
ProviderId.GOOGLE,
298-
undefined,
299-
OperationType.LINK
300-
);
282+
providerId: ProviderId.GOOGLE,
283+
operationType: OperationType.LINK
284+
});
301285
idpStubs._link.returns(Promise.resolve(cred));
302286
const promise = linkWithPopup(user, provider, resolver);
303287
iframeEvent({
@@ -317,12 +301,11 @@ describe('src/core/strategies/popup', () => {
317301
});
318302

319303
it('does not error if the poll timeout trips', async () => {
320-
const cred = new UserCredentialImpl(
304+
const cred = new UserCredentialImpl({
321305
user,
322-
ProviderId.GOOGLE,
323-
undefined,
324-
OperationType.LINK
325-
);
306+
providerId: ProviderId.GOOGLE,
307+
operationType: OperationType.LINK
308+
});
326309
idpStubs._link.returns(Promise.resolve(cred));
327310
const promise = linkWithPopup(user, provider, resolver);
328311
delay(() => {
@@ -336,12 +319,11 @@ describe('src/core/strategies/popup', () => {
336319
});
337320

338321
it('does error if the poll timeout and event timeout trip', async () => {
339-
const cred = new UserCredentialImpl(
322+
const cred = new UserCredentialImpl({
340323
user,
341-
ProviderId.GOOGLE,
342-
undefined,
343-
OperationType.LINK
344-
);
324+
providerId: ProviderId.GOOGLE,
325+
operationType: OperationType.LINK
326+
});
345327
idpStubs._link.returns(Promise.resolve(cred));
346328
const promise = linkWithPopup(user, provider, resolver);
347329
delay(() => {
@@ -379,12 +361,11 @@ describe('src/core/strategies/popup', () => {
379361
});
380362

381363
it('cancels the task if called consecutively', async () => {
382-
const cred = new UserCredentialImpl(
364+
const cred = new UserCredentialImpl({
383365
user,
384-
ProviderId.GOOGLE,
385-
undefined,
386-
OperationType.LINK
387-
);
366+
providerId: ProviderId.GOOGLE,
367+
operationType: OperationType.LINK
368+
});
388369
idpStubs._link.returns(Promise.resolve(cred));
389370
const firstPromise = linkWithPopup(user, provider, resolver);
390371
const secondPromise = linkWithPopup(user, provider, resolver);
@@ -406,12 +387,11 @@ describe('src/core/strategies/popup', () => {
406387
});
407388

408389
it('completes the full flow', async () => {
409-
const cred = new UserCredentialImpl(
390+
const cred = new UserCredentialImpl({
410391
user,
411-
ProviderId.GOOGLE,
412-
undefined,
413-
OperationType.REAUTHENTICATE
414-
);
392+
providerId: ProviderId.GOOGLE,
393+
operationType: OperationType.REAUTHENTICATE
394+
});
415395
idpStubs._reauth.returns(Promise.resolve(cred));
416396
const promise = reauthenticateWithPopup(user, provider, resolver);
417397
iframeEvent({
@@ -421,12 +401,11 @@ describe('src/core/strategies/popup', () => {
421401
});
422402

423403
it('ignores events for another event id', async () => {
424-
const cred = new UserCredentialImpl(
404+
const cred = new UserCredentialImpl({
425405
user,
426-
ProviderId.GOOGLE,
427-
undefined,
428-
OperationType.REAUTHENTICATE
429-
);
406+
providerId: ProviderId.GOOGLE,
407+
operationType: OperationType.REAUTHENTICATE
408+
});
430409
idpStubs._reauth.returns(Promise.resolve(cred));
431410
const promise = reauthenticateWithPopup(user, provider, resolver);
432411
iframeEvent({
@@ -447,12 +426,11 @@ describe('src/core/strategies/popup', () => {
447426
});
448427

449428
it('does not call idp tasks if event is error', async () => {
450-
const cred = new UserCredentialImpl(
429+
const cred = new UserCredentialImpl({
451430
user,
452-
ProviderId.GOOGLE,
453-
undefined,
454-
OperationType.REAUTHENTICATE
455-
);
431+
providerId: ProviderId.GOOGLE,
432+
operationType: OperationType.REAUTHENTICATE
433+
});
456434
idpStubs._reauth.returns(Promise.resolve(cred));
457435
const promise = reauthenticateWithPopup(user, provider, resolver);
458436
iframeEvent({
@@ -472,12 +450,11 @@ describe('src/core/strategies/popup', () => {
472450
});
473451

474452
it('does not error if the poll timeout trips', async () => {
475-
const cred = new UserCredentialImpl(
453+
const cred = new UserCredentialImpl({
476454
user,
477-
ProviderId.GOOGLE,
478-
undefined,
479-
OperationType.REAUTHENTICATE
480-
);
455+
providerId: ProviderId.GOOGLE,
456+
operationType: OperationType.REAUTHENTICATE
457+
});
481458
idpStubs._reauth.returns(Promise.resolve(cred));
482459
const promise = reauthenticateWithPopup(user, provider, resolver);
483460
delay(() => {
@@ -491,12 +468,11 @@ describe('src/core/strategies/popup', () => {
491468
});
492469

493470
it('does error if the poll timeout and event timeout trip', async () => {
494-
const cred = new UserCredentialImpl(
471+
const cred = new UserCredentialImpl({
495472
user,
496-
ProviderId.GOOGLE,
497-
undefined,
498-
OperationType.REAUTHENTICATE
499-
);
473+
providerId: ProviderId.GOOGLE,
474+
operationType: OperationType.REAUTHENTICATE
475+
});
500476
idpStubs._reauth.returns(Promise.resolve(cred));
501477
const promise = reauthenticateWithPopup(user, provider, resolver);
502478
delay(() => {
@@ -534,12 +510,11 @@ describe('src/core/strategies/popup', () => {
534510
});
535511

536512
it('cancels the task if called consecutively', async () => {
537-
const cred = new UserCredentialImpl(
513+
const cred = new UserCredentialImpl({
538514
user,
539-
ProviderId.GOOGLE,
540-
undefined,
541-
OperationType.REAUTHENTICATE
542-
);
515+
providerId: ProviderId.GOOGLE,
516+
operationType: OperationType.REAUTHENTICATE
517+
});
543518
idpStubs._reauth.returns(Promise.resolve(cred));
544519
const firstPromise = reauthenticateWithPopup(user, provider, resolver);
545520
const secondPromise = reauthenticateWithPopup(user, provider, resolver);

0 commit comments

Comments
 (0)