Skip to content

Commit ac019ae

Browse files
EhespEhesp
authored
Merge pull request #1220 from firebase/@invertase/behavior-state
2 parents 2f0b2c2 + 68e3df8 commit ac019ae

File tree

7 files changed

+29
-52
lines changed

7 files changed

+29
-52
lines changed

‎packages/core/src/auth.test.ts‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,8 @@ describe("signInWithEmailAndPassword", () => {
114114
expect(mockBehavior).toHaveBeenCalledWith(mockUI, credential);
115115
expect(result.providerId).toBe("password");
116116

117-
// Only the `finally` block is called here.
118-
expect(vi.mocked(mockUI.setState).mock.calls).toEqual([["idle"]]);
117+
// Auth method sets pending at start, then idle in finally block.
118+
expect(vi.mocked(mockUI.setState).mock.calls).toEqual([["pending"], ["idle"]]);
119119
});
120120

121121
it("should call the autoUpgradeAnonymousCredential behavior if enabled and handle no result from the behavior", async () => {
@@ -211,8 +211,8 @@ describe("createUserWithEmailAndPassword", () => {
211211
expect(mockBehavior).toHaveBeenCalledWith(mockUI, credential);
212212
expect(result.providerId).toBe("password");
213213

214-
// Only the `finally` block is called here.
215-
expect(vi.mocked(mockUI.setState).mock.calls).toEqual([["idle"]]);
214+
// Auth method sets pending at start, then idle in finally block.
215+
expect(vi.mocked(mockUI.setState).mock.calls).toEqual([["pending"], ["idle"]]);
216216
});
217217

218218
it("should call the autoUpgradeAnonymousCredential behavior if enabled and handle no result from the behavior", async () => {
@@ -497,8 +497,8 @@ describe("confirmPhoneNumber", () => {
497497
expect(mockBehavior).toHaveBeenCalledWith(mockUI, credential);
498498
expect(result.providerId).toBe("phone");
499499

500-
// Only the `finally` block is called here.
501-
expect(vi.mocked(mockUI.setState).mock.calls).toEqual([["idle"]]);
500+
// Auth method sets pending at start, then idle in finally block.
501+
expect(vi.mocked(mockUI.setState).mock.calls).toEqual([["pending"], ["idle"]]);
502502
});
503503

504504
it("should not call behavior when user is not anonymous", async () => {
@@ -770,8 +770,8 @@ describe("signInWithEmailLink", () => {
770770
expect(mockBehavior).toHaveBeenCalledWith(mockUI, credential);
771771
expect(result.providerId).toBe("emailLink");
772772

773-
// Only the `finally` block is called here.
774-
expect(vi.mocked(mockUI.setState).mock.calls).toEqual([["idle"]]);
773+
// Auth method sets pending at start, then idle in finally block.
774+
expect(vi.mocked(mockUI.setState).mock.calls).toEqual([["pending"], ["idle"]]);
775775
});
776776

777777
it("should call the autoUpgradeAnonymousCredential behavior if enabled and handle no result from the behavior", async () => {
@@ -868,8 +868,8 @@ describe("signInWithCredential", () => {
868868
expect(mockBehavior).toHaveBeenCalledWith(mockUI, credential);
869869
expect(result.providerId).toBe("password");
870870

871-
// Only the `finally` block is called here.
872-
expect(vi.mocked(mockUI.setState).mock.calls).toEqual([["idle"]]);
871+
// Auth method sets pending at start, then idle in finally block.
872+
expect(vi.mocked(mockUI.setState).mock.calls).toEqual([["pending"], ["idle"]]);
873873
});
874874

875875
it("should call the autoUpgradeAnonymousCredential behavior if enabled and handle no result from the behavior", async () => {
@@ -923,7 +923,7 @@ describe("signInWithCredential", () => {
923923

924924
expect(handleFirebaseError).toHaveBeenCalledWith(mockUI, error);
925925

926-
expect(vi.mocked(mockUI.setState).mock.calls).toEqual([["idle"]]);
926+
expect(vi.mocked(mockUI.setState).mock.calls).toEqual([["pending"], ["idle"]]);
927927

928928
expect(_signInWithCredential).not.toHaveBeenCalled();
929929
});
@@ -1050,7 +1050,7 @@ describe("signInWithProvider", () => {
10501050
await signInWithProvider(mockUI, provider);
10511051

10521052
expect(handleFirebaseError).toHaveBeenCalledWith(mockUI, error);
1053-
expect(vi.mocked(mockUI.setState).mock.calls).toEqual([["idle"]]);
1053+
expect(vi.mocked(mockUI.setState).mock.calls).toEqual([["pending"], ["idle"]]);
10541054
});
10551055
});
10561056

‎packages/core/src/auth.ts‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,13 @@ import { hasBehavior, getBehavior } from "./behaviors/index";
4040
import { FirebaseError } from "firebase/app";
4141
import { getTranslation } from "./translations";
4242

43-
async function handlePendingCredential(ui: FirebaseUIConfiguration, user: UserCredential): Promise<UserCredential> {
43+
async function handlePendingCredential(_ui: FirebaseUIConfiguration, user: UserCredential): Promise<UserCredential> {
4444
const pendingCredString = window.sessionStorage.getItem("pendingCred");
4545
if (!pendingCredString) return user;
4646

4747
try {
4848
const pendingCred = JSON.parse(pendingCredString);
49-
ui.setState("pending");
5049
const result = await linkWithCredential(user.user, pendingCred);
51-
ui.setState("idle");
5250
window.sessionStorage.removeItem("pendingCred");
5351
return result;
5452
} catch {
@@ -63,6 +61,7 @@ export async function signInWithEmailAndPassword(
6361
password: string
6462
): Promise<UserCredential> {
6563
try {
64+
ui.setState("pending");
6665
const credential = EmailAuthProvider.credential(email, password);
6766

6867
if (hasBehavior(ui, "autoUpgradeAnonymousCredential")) {
@@ -73,7 +72,6 @@ export async function signInWithEmailAndPassword(
7372
}
7473
}
7574

76-
ui.setState("pending");
7775
const result = await _signInWithCredential(ui.auth, credential);
7876
return handlePendingCredential(ui, result);
7977
} catch (error) {
@@ -90,6 +88,7 @@ export async function createUserWithEmailAndPassword(
9088
displayName?: string
9189
): Promise<UserCredential> {
9290
try {
91+
ui.setState("pending");
9392
const credential = EmailAuthProvider.credential(email, password);
9493

9594
if (hasBehavior(ui, "requireDisplayName") && !displayName) {
@@ -108,7 +107,6 @@ export async function createUserWithEmailAndPassword(
108107
}
109108
}
110109

111-
ui.setState("pending");
112110
const result = await _createUserWithEmailAndPassword(ui.auth, email, password);
113111

114112
if (hasBehavior(ui, "requireDisplayName")) {
@@ -144,6 +142,7 @@ export async function confirmPhoneNumber(
144142
verificationCode: string
145143
): Promise<UserCredential> {
146144
try {
145+
ui.setState("pending");
147146
const currentUser = ui.auth.currentUser;
148147
const credential = PhoneAuthProvider.credential(confirmationResult.verificationId, verificationCode);
149148

@@ -155,7 +154,6 @@ export async function confirmPhoneNumber(
155154
}
156155
}
157156

158-
ui.setState("pending");
159157
const result = await _signInWithCredential(ui.auth, credential);
160158
return handlePendingCredential(ui, result);
161159
} catch (error) {
@@ -178,13 +176,13 @@ export async function sendPasswordResetEmail(ui: FirebaseUIConfiguration, email:
178176

179177
export async function sendSignInLinkToEmail(ui: FirebaseUIConfiguration, email: string): Promise<void> {
180178
try {
179+
ui.setState("pending");
181180
const actionCodeSettings = {
182181
url: window.location.href,
183182
// TODO(ehesp): Check this...
184183
handleCodeInApp: true,
185184
} satisfies ActionCodeSettings;
186185

187-
ui.setState("pending");
188186
await _sendSignInLinkToEmail(ui.auth, email, actionCodeSettings);
189187
// TODO: Should this be a behavior ("storageStrategy")?
190188
window.localStorage.setItem("emailForSignIn", email);
@@ -209,6 +207,7 @@ export async function signInWithCredential(
209207
credential: AuthCredential
210208
): Promise<UserCredential> {
211209
try {
210+
ui.setState("pending");
212211
if (hasBehavior(ui, "autoUpgradeAnonymousCredential")) {
213212
const userCredential = await getBehavior(ui, "autoUpgradeAnonymousCredential")(ui, credential);
214213

@@ -219,7 +218,6 @@ export async function signInWithCredential(
219218
}
220219
}
221220

222-
ui.setState("pending");
223221
const result = await _signInWithCredential(ui.auth, credential);
224222
return handlePendingCredential(ui, result);
225223
} catch (error) {
@@ -246,6 +244,7 @@ export async function signInWithProvider(
246244
provider: AuthProvider
247245
): Promise<UserCredential | never> {
248246
try {
247+
ui.setState("pending");
249248
if (hasBehavior(ui, "autoUpgradeAnonymousProvider")) {
250249
const credential = await getBehavior(ui, "autoUpgradeAnonymousProvider")(ui, provider);
251250

@@ -283,7 +282,6 @@ export async function completeEmailLinkSignIn(
283282

284283
ui.setState("pending");
285284
const result = await signInWithEmailLink(ui, email, currentUrl);
286-
ui.setState("idle"); // TODO(ehesp): Do we need this here?
287285
return handlePendingCredential(ui, result);
288286
} catch (error) {
289287
handleFirebaseError(ui, error);

‎packages/core/src/behaviors/anonymous-upgrade.test.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,6 @@ describe("autoUpgradeAnonymousCredentialHandler", () => {
4343
const result = await autoUpgradeAnonymousCredentialHandler(mockUI, mockCredential);
4444

4545
expect(linkWithCredential).toHaveBeenCalledWith(mockUser, mockCredential);
46-
expect(mockUI.setState).toHaveBeenCalledWith("pending");
47-
expect(mockUI.setState).toHaveBeenCalledWith("idle");
4846
expect(result).toBe(mockResult);
4947
});
5048

‎packages/core/src/behaviors/anonymous-upgrade.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,12 @@ export const autoUpgradeAnonymousCredentialHandler = async (
2121

2222
const oldUserId = currentUser.uid;
2323

24-
ui.setState("pending");
2524
const result = await linkWithCredential(currentUser, credential);
2625

2726
if (onUpgrade) {
2827
await onUpgrade(ui, oldUserId, result);
2928
}
3029

31-
ui.setState("idle");
3230
return result;
3331
};
3432

‎packages/core/src/behaviors/auto-anonymous-login.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { InitHandler } from "./utils";
33

44
export const autoAnonymousLoginHandler: InitHandler = async (ui) => {
55
const auth = ui.auth;
6+
67
if (!auth.currentUser) {
78
await signInAnonymously(auth);
89
}

‎packages/core/src/behaviors/provider-strategy.test.ts‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,12 @@ describe("signInWithRediectHandler", () => {
3838

3939
await signInWithRediectHandler(mockUI, mockProvider);
4040

41-
expect(mockUI.setState).toHaveBeenCalledWith("pending");
4241
expect(signInWithRedirect).toHaveBeenCalledWith(mockAuth, mockProvider);
4342
});
4443
});
4544

4645
describe("signInWithPopupHandler", () => {
47-
it("should set state to pending, call signInWithPopup, set state to idle, and return result", async () => {
46+
it("should call signInWithPopup and return result", async () => {
4847
const mockAuth = {} as Auth;
4948
const mockUI = createMockUI({ auth: mockAuth });
5049
const mockProvider = { providerId: "google.com" } as AuthProvider;
@@ -54,13 +53,11 @@ describe("signInWithPopupHandler", () => {
5453

5554
const result = await signInWithPopupHandler(mockUI, mockProvider);
5655

57-
expect(mockUI.setState).toHaveBeenCalledWith("pending");
5856
expect(signInWithPopup).toHaveBeenCalledWith(mockAuth, mockProvider);
59-
expect(mockUI.setState).toHaveBeenCalledWith("idle");
6057
expect(result).toBe(mockResult);
6158
});
6259

63-
it("should not set state to idle when signInWithPopup fails", async () => {
60+
it("should throw error when signInWithPopup fails", async () => {
6461
const mockAuth = {} as Auth;
6562
const mockUI = createMockUI({ auth: mockAuth });
6663
const mockProvider = { providerId: "google.com" } as AuthProvider;
@@ -69,13 +66,11 @@ describe("signInWithPopupHandler", () => {
6966
vi.mocked(signInWithPopup).mockRejectedValue(mockError);
7067

7168
await expect(signInWithPopupHandler(mockUI, mockProvider)).rejects.toThrow("Popup sign in failed");
72-
expect(mockUI.setState).toHaveBeenCalledWith("pending");
73-
expect(mockUI.setState).not.toHaveBeenCalledWith("idle");
7469
});
7570
});
7671

7772
describe("linkWithRedirectHandler", () => {
78-
it("should set state to pending and call linkWithRedirect", async () => {
73+
it("should call linkWithRedirect", async () => {
7974
const mockAuth = {} as Auth;
8075
const mockUI = createMockUI({ auth: mockAuth });
8176
const mockUser = { uid: "test-user" } as User;
@@ -85,13 +80,12 @@ describe("linkWithRedirectHandler", () => {
8580

8681
await linkWithRedirectHandler(mockUI, mockUser, mockProvider);
8782

88-
expect(mockUI.setState).toHaveBeenCalledWith("pending");
8983
expect(linkWithRedirect).toHaveBeenCalledWith(mockUser, mockProvider);
9084
});
9185
});
9286

9387
describe("linkWithPopupHandler", () => {
94-
it("should set state to pending, call linkWithPopup, set state to idle, and return result", async () => {
88+
it("should call linkWithPopup and return result", async () => {
9589
const mockAuth = {} as Auth;
9690
const mockUI = createMockUI({ auth: mockAuth });
9791
const mockUser = { uid: "test-user" } as User;
@@ -102,13 +96,11 @@ describe("linkWithPopupHandler", () => {
10296

10397
const result = await linkWithPopupHandler(mockUI, mockUser, mockProvider);
10498

105-
expect(mockUI.setState).toHaveBeenCalledWith("pending");
10699
expect(linkWithPopup).toHaveBeenCalledWith(mockUser, mockProvider);
107-
expect(mockUI.setState).toHaveBeenCalledWith("idle");
108100
expect(result).toBe(mockResult);
109101
});
110102

111-
it("should not set state to idle when linkWithPopup fails", async () => {
103+
it("should throw error when linkWithPopup fails", async () => {
112104
const mockAuth = {} as Auth;
113105
const mockUI = createMockUI({ auth: mockAuth });
114106
const mockUser = { uid: "test-user" } as User;
@@ -118,7 +110,5 @@ describe("linkWithPopupHandler", () => {
118110
vi.mocked(linkWithPopup).mockRejectedValue(mockError);
119111

120112
await expect(linkWithPopupHandler(mockUI, mockUser, mockProvider)).rejects.toThrow("Popup link failed");
121-
expect(mockUI.setState).toHaveBeenCalledWith("pending");
122-
expect(mockUI.setState).not.toHaveBeenCalledWith("idle");
123113
});
124114
});

‎packages/core/src/behaviors/provider-strategy.ts‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,25 +20,17 @@ export type ProviderLinkStrategyHandler = (
2020
) => Promise<never | UserCredential>;
2121

2222
export const signInWithRediectHandler: ProviderSignInStrategyHandler = async (ui, provider) => {
23-
ui.setState("pending");
2423
return signInWithRedirect(ui.auth, provider);
2524
};
2625

2726
export const signInWithPopupHandler: ProviderSignInStrategyHandler = async (ui, provider) => {
28-
ui.setState("pending");
29-
const result = await signInWithPopup(ui.auth, provider);
30-
ui.setState("idle");
31-
return result;
27+
return signInWithPopup(ui.auth, provider);
3228
};
3329

34-
export const linkWithRedirectHandler: ProviderLinkStrategyHandler = async (ui, user, provider) => {
35-
ui.setState("pending");
30+
export const linkWithRedirectHandler: ProviderLinkStrategyHandler = async (_ui, user, provider) => {
3631
return linkWithRedirect(user, provider);
3732
};
3833

39-
export const linkWithPopupHandler: ProviderLinkStrategyHandler = async (ui, user, provider) => {
40-
ui.setState("pending");
41-
const result = await linkWithPopup(user, provider);
42-
ui.setState("idle");
43-
return result;
34+
export const linkWithPopupHandler: ProviderLinkStrategyHandler = async (_ui, user, provider) => {
35+
return linkWithPopup(user, provider);
4436
};

0 commit comments

Comments
 (0)