Skip to content

Commit fd281d7

Browse files
committed
refactor(auth): simplify loading state to counter-based approach
Replace per-method Set<AuthMethod> with a simple pendingOps counter. Each child reports busy state via onLoadingChange; counter handles overlapping operations correctly (e.g., rapid double-click edge case). - Rename onSuccess to onAuthComplete in MethodSelection for clarity - Fix React.FormEvent to use imported FormEvent type - Add no-redirect safety check in SocialLogin
1 parent 01e99c2 commit fd281d7

File tree

5 files changed

+42
-55
lines changed

5 files changed

+42
-55
lines changed

apps/app/components/auth/auth-form.tsx

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import { Button, Input, cn } from "@repo/ui";
22
import { Link } from "@tanstack/react-router";
33
import { ArrowLeft, Mail } from "lucide-react";
4-
import type { ComponentProps } from "react";
4+
import type { ComponentProps, FormEvent } from "react";
55
import { OtpVerification } from "./otp-verification";
66
import { PasskeyLogin } from "./passkey-login";
77
import { SocialLogin } from "./social-login";
8-
import { useAuthForm, type AuthMethod } from "./use-auth-form";
8+
import { useAuthForm } from "./use-auth-form";
99

1010
const APP_NAME = import.meta.env.VITE_APP_NAME || "your account";
1111

@@ -65,7 +65,7 @@ export function AuthForm({
6565
goToEmailStep,
6666
goToMethodStep,
6767
resetToEmail,
68-
setMethodLoading,
68+
setChildBusy,
6969
mode: formMode,
7070
} = useAuthForm({
7171
onSuccess,
@@ -112,9 +112,9 @@ export function AuthForm({
112112
isSignup={isSignup}
113113
isDisabled={isDisabled}
114114
onEmailClick={goToEmailStep}
115-
onSuccess={completeAuth}
115+
onAuthComplete={completeAuth}
116116
onError={handleError}
117-
setMethodLoading={setMethodLoading}
117+
onLoadingChange={setChildBusy}
118118
returnTo={returnTo}
119119
/>
120120
)}
@@ -138,7 +138,7 @@ export function AuthForm({
138138
isDisabled={isDisabled}
139139
onSuccess={completeAuth}
140140
onError={handleError}
141-
setMethodLoading={setMethodLoading}
141+
onLoadingChange={setChildBusy}
142142
onBack={handleOtpBack}
143143
onCancel={resetToEmail}
144144
/>
@@ -152,19 +152,19 @@ interface MethodSelectionProps {
152152
isSignup: boolean;
153153
isDisabled: boolean;
154154
onEmailClick: () => void;
155-
onSuccess: () => void;
155+
onAuthComplete: () => void;
156156
onError: (error: string | null) => void;
157-
setMethodLoading: (method: AuthMethod, loading: boolean) => void;
157+
onLoadingChange: (loading: boolean) => void;
158158
returnTo?: string;
159159
}
160160

161161
function MethodSelection({
162162
isSignup,
163163
isDisabled,
164164
onEmailClick,
165-
onSuccess,
165+
onAuthComplete,
166166
onError,
167-
setMethodLoading,
167+
onLoadingChange,
168168
returnTo,
169169
}: MethodSelectionProps) {
170170
const heading = isSignup ? "Create your account" : `Log in to ${APP_NAME}`;
@@ -177,7 +177,7 @@ function MethodSelection({
177177
<SocialLogin
178178
onError={onError}
179179
isDisabled={isDisabled}
180-
onLoadingChange={(loading) => setMethodLoading("social", loading)}
180+
onLoadingChange={onLoadingChange}
181181
returnTo={returnTo}
182182
/>
183183

@@ -195,9 +195,9 @@ function MethodSelection({
195195
{/* Passkey only available for login (requires existing account) */}
196196
{!isSignup && (
197197
<PasskeyLogin
198-
onSuccess={onSuccess}
198+
onSuccess={onAuthComplete}
199199
onError={onError}
200-
onLoadingChange={(loading) => setMethodLoading("passkey", loading)}
200+
onLoadingChange={onLoadingChange}
201201
isDisabled={isDisabled}
202202
/>
203203
)}
@@ -239,7 +239,7 @@ interface EmailInputProps {
239239
isSignup: boolean;
240240
isDisabled: boolean;
241241
onEmailChange: (email: string) => void;
242-
onSubmit: (e?: React.FormEvent) => void;
242+
onSubmit: (e?: FormEvent) => void;
243243
onBack: () => void;
244244
}
245245

@@ -300,7 +300,7 @@ interface OtpStepProps {
300300
isDisabled: boolean;
301301
onSuccess: () => void;
302302
onError: (error: string | null) => void;
303-
setMethodLoading: (method: AuthMethod, loading: boolean) => void;
303+
onLoadingChange: (loading: boolean) => void;
304304
onBack: () => void;
305305
onCancel: () => void;
306306
}
@@ -310,7 +310,7 @@ function OtpStep({
310310
isDisabled,
311311
onSuccess,
312312
onError,
313-
setMethodLoading,
313+
onLoadingChange,
314314
onBack,
315315
onCancel,
316316
}: OtpStepProps) {
@@ -327,7 +327,7 @@ function OtpStep({
327327
email={email}
328328
onSuccess={onSuccess}
329329
onError={onError}
330-
onLoadingChange={(loading) => setMethodLoading("otp", loading)}
330+
onLoadingChange={onLoadingChange}
331331
onCancel={onCancel}
332332
isDisabled={isDisabled}
333333
/>

apps/app/components/auth/otp-verification.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,18 @@ interface OtpVerificationProps {
1616
email: string;
1717
onSuccess: () => void;
1818
onError: (error: string | null) => void;
19+
onLoadingChange?: (loading: boolean) => void;
1920
onCancel: () => void;
2021
isDisabled?: boolean;
21-
onLoadingChange?: (loading: boolean) => void;
2222
}
2323

2424
export function OtpVerification({
2525
email,
2626
onSuccess,
2727
onError,
28+
onLoadingChange,
2829
onCancel,
2930
isDisabled,
30-
onLoadingChange,
3131
}: OtpVerificationProps) {
3232
const [otp, setOtp] = useState("");
3333
const [isLoading, setIsLoading] = useState(false);
@@ -85,7 +85,7 @@ export function OtpVerification({
8585
}
8686
};
8787

88-
const handleResendOtp = useCallback(async () => {
88+
const handleResendOtp = async () => {
8989
if (resendCooldown > 0) return;
9090

9191
setOtp("");
@@ -111,7 +111,7 @@ export function OtpVerification({
111111
} finally {
112112
setLoading(false);
113113
}
114-
}, [email, onError, resendCooldown, setLoading]);
114+
};
115115

116116
const disabled = isDisabled || isLoading;
117117

apps/app/components/auth/passkey-login.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import { useCallback, useEffect, useRef, useState } from "react";
77
interface PasskeyLoginProps {
88
onSuccess: () => void;
99
onError: (error: string | null) => void;
10-
isDisabled?: boolean;
1110
onLoadingChange?: (loading: boolean) => void;
11+
isDisabled?: boolean;
1212
}
1313

1414
/**
@@ -20,8 +20,8 @@ interface PasskeyLoginProps {
2020
export function PasskeyLogin({
2121
onSuccess,
2222
onError,
23-
isDisabled,
2423
onLoadingChange,
24+
isDisabled,
2525
}: PasskeyLoginProps) {
2626
const [isLoading, setIsLoading] = useState(false);
2727

@@ -32,6 +32,7 @@ export function PasskeyLogin({
3232
},
3333
[onLoadingChange],
3434
);
35+
3536
const onSuccessRef = useRef(onSuccess);
3637
onSuccessRef.current = onSuccess;
3738

apps/app/components/auth/social-login.tsx

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,16 @@ import { useCallback, useState } from "react";
77
interface SocialLoginProps {
88
onError: (error: string | null) => void;
99
isDisabled?: boolean;
10+
onLoadingChange?: (loading: boolean) => void;
1011
/** Post-auth redirect destination (already validated by caller). */
1112
returnTo?: string;
12-
onLoadingChange?: (loading: boolean) => void;
1313
}
1414

1515
export function SocialLogin({
1616
onError,
1717
isDisabled,
18-
returnTo,
1918
onLoadingChange,
19+
returnTo,
2020
}: SocialLoginProps) {
2121
const queryClient = useQueryClient();
2222
const [isLoading, setIsLoading] = useState(false);
@@ -47,12 +47,14 @@ export function SocialLogin({
4747
callbackURL,
4848
});
4949

50-
// Handle error result (Better Auth returns { error } instead of throwing)
5150
if (result?.error) {
5251
onError(result.error.message || "Failed to sign in with Google");
5352
setLoading(false);
53+
} else if (!result?.data?.redirect) {
54+
// No redirect (popup blocked, misconfigured provider, etc.) - reset loading
55+
setLoading(false);
5456
}
55-
// On success, page redirects - component unmounts, no cleanup needed
57+
// On redirect, page navigates away - component unmounts, no cleanup needed
5658
} catch (err) {
5759
console.error("Google login error:", err);
5860
onError("Failed to sign in with Google");

apps/app/components/auth/use-auth-form.ts

Lines changed: 12 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,8 @@ import { useCallback, useRef, useState } from "react";
44

55
export type AuthStep = "method" | "email" | "otp";
66

7-
/** Authentication method identifiers for tracking concurrent loading states */
8-
export type AuthMethod = "social" | "passkey" | "otp";
9-
107
// Minimal state machine for passwordless OTP flow. Intentionally shallow:
118
// - Errors are orthogonal to steps (can occur at any step)
12-
// - Loading states handled by isLoading/isChildLoading
139
// - No terminal state (component unmounts on success)
1410
// Revisit if adding password fallback or MFA steps.
1511
const VALID_TRANSITIONS: Record<AuthStep, AuthStep[]> = {
@@ -40,37 +36,25 @@ export function useAuthForm({
4036
const [step, setStep] = useState<AuthStep>("method");
4137
const [email, setEmail] = useState("");
4238
const [isLoading, setIsLoading] = useState(false);
43-
// Tracks loading state per auth method to handle concurrent attempts
44-
const [loadingMethods, setLoadingMethods] = useState<Set<AuthMethod>>(
45-
() => new Set(),
46-
);
39+
// Counter-based to handle overlapping child operations (e.g., rapid double-click)
40+
const [pendingOps, setPendingOps] = useState(0);
4741
const [error, setError] = useState<string | null>(null);
4842

49-
const isMethodLoading = loadingMethods.size > 0;
50-
// Guards against concurrent auth completion (e.g., passkey conditional UI + manual OTP).
43+
// Guards against concurrent auth completion (e.g., passkey conditional UI + manual click).
44+
// Conditional passkey autofill intentionally doesn't block UI - it's passive/background.
5145
// Reset when returning to method step to allow retry after navigation back.
5246
const hasSucceededRef = useRef(false);
5347
// Sync ref for checking current step in transitionTo without stale closure
5448
const stepRef = useRef(step);
5549
stepRef.current = step;
5650

57-
// Unified busy state: parent loading, method loading, or external loading
58-
const isDisabled = isLoading || isMethodLoading || !!isExternallyLoading;
59-
60-
// Keyed loading tracker - idempotent to handle redundant calls
61-
const setMethodLoading = useCallback(
62-
(method: AuthMethod, loading: boolean) => {
63-
setLoadingMethods((prev) => {
64-
const has = prev.has(method);
65-
if (loading ? has : !has) return prev; // No logical change
66-
const next = new Set(prev);
67-
if (loading) next.add(method);
68-
else next.delete(method);
69-
return next;
70-
});
71-
},
72-
[],
73-
);
51+
// Track child loading via counter to correctly handle overlapping operations
52+
const setChildBusy = useCallback((busy: boolean) => {
53+
setPendingOps((c) => (busy ? c + 1 : Math.max(0, c - 1)));
54+
}, []);
55+
56+
// Unified busy state: disables navigation and other auth methods while any flow is active
57+
const isDisabled = isLoading || pendingOps > 0 || !!isExternallyLoading;
7458

7559
const completeAuth = async () => {
7660
if (hasSucceededRef.current) return;
@@ -169,6 +153,6 @@ export function useAuthForm({
169153
goToEmailStep,
170154
goToMethodStep,
171155
resetToEmail,
172-
setMethodLoading,
156+
setChildBusy,
173157
};
174158
}

0 commit comments

Comments
 (0)