diff --git a/packages/react/src/auth/screens/sign-up-auth-screen.test.tsx b/packages/react/src/auth/screens/sign-up-auth-screen.test.tsx index 3cb14217a..083f3c8a5 100644 --- a/packages/react/src/auth/screens/sign-up-auth-screen.test.tsx +++ b/packages/react/src/auth/screens/sign-up-auth-screen.test.tsx @@ -21,8 +21,20 @@ import { registerLocale } from "@firebase-oss/ui-translations"; import { MultiFactorResolver, type User } from "firebase/auth"; vi.mock("~/auth/forms/sign-up-auth-form", () => ({ - SignUpAuthForm: ({ onSignInClick }: { onSignInClick?: () => void }) => ( + SignUpAuthForm: ({ + onSignUp, + onSignInClick, + }: { + onSignUp?: (credential: any) => void; + onSignInClick?: () => void; + }) => (
+ @@ -294,7 +306,31 @@ describe("", () => { expect(onSignUp).toHaveBeenCalledWith(mockUser); }); - it("calls onSignUp when user authenticates via useOnUserAuthenticated hook", () => { + it("calls onSignUp from the resolved form credential", () => { + const onSignUp = vi.fn(); + + const mockAuth = { + onAuthStateChanged: vi.fn(() => vi.fn()), + }; + + const ui = createMockUI({ + auth: mockAuth as any, + }); + + render( + + + + ); + + fireEvent.click(screen.getByTestId("sign-up-success-button")); + + expect(onSignUp).toHaveBeenCalledTimes(1); + expect(onSignUp).toHaveBeenCalledWith({ uid: "signup-form-user", isAnonymous: false }); + expect(mockAuth.onAuthStateChanged).not.toHaveBeenCalled(); + }); + + it("calls onSignUp from auth state for child-based sign-up flows", () => { const onSignUp = vi.fn(); let authStateChangeCallback: ((user: User | null) => void) | null = null; @@ -311,14 +347,16 @@ describe("", () => { render( - + +
+ ); expect(mockAuth.onAuthStateChanged).toHaveBeenCalledTimes(1); const mockUser = { - uid: "test-user-id", + uid: "child-provider-user", isAnonymous: false, } as User; @@ -330,6 +368,42 @@ describe("", () => { expect(onSignUp).toHaveBeenCalledWith(mockUser); }); + it("dedupes the same user across form and auth state paths", () => { + const onSignUp = vi.fn(); + let authStateChangeCallback: ((user: User | null) => void) | null = null; + + const mockAuth = { + onAuthStateChanged: vi.fn((callback: (user: User | null) => void) => { + authStateChangeCallback = callback; + return vi.fn(); + }), + }; + + const ui = createMockUI({ + auth: mockAuth as any, + }); + + render( + + +
+ + + ); + + fireEvent.click(screen.getByTestId("sign-up-success-button")); + + act(() => { + authStateChangeCallback!({ + uid: "signup-form-user", + isAnonymous: false, + } as User); + }); + + expect(onSignUp).toHaveBeenCalledTimes(1); + expect(onSignUp).toHaveBeenCalledWith({ uid: "signup-form-user", isAnonymous: false }); + }); + it("does not call onSignUp for anonymous users", () => { const onSignUp = vi.fn(); let authStateChangeCallback: ((user: User | null) => void) | null = null; @@ -347,7 +421,9 @@ describe("", () => { render( - + +
+ ); diff --git a/packages/react/src/auth/screens/sign-up-auth-screen.tsx b/packages/react/src/auth/screens/sign-up-auth-screen.tsx index 433af8fb9..804f7582f 100644 --- a/packages/react/src/auth/screens/sign-up-auth-screen.tsx +++ b/packages/react/src/auth/screens/sign-up-auth-screen.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import type { PropsWithChildren } from "react"; +import { type PropsWithChildren, useCallback, useRef } from "react"; import type { User } from "firebase/auth"; import { Divider } from "~/components/divider"; import { useOnUserAuthenticated, useUI } from "~/hooks"; @@ -39,11 +39,28 @@ export type SignUpAuthScreenProps = PropsWithChildren(null); const titleText = getTranslation(ui, "labels", "signUp"); const subtitleText = getTranslation(ui, "prompts", "enterDetailsToCreate"); - useOnUserAuthenticated(onSignUp); + const handleSignUp = useCallback( + (user: User) => { + if (handledUserIdRef.current === user.uid) { + return; + } + + handledUserIdRef.current = user.uid; + onSignUp?.(user); + }, + [onSignUp] + ); + + // The built-in email/password form can report success from the resolved credential, + // which is the earliest point where requireDisplayName() has finished updating the profile. + // We keep the auth-state listener only for flows that complete outside that callback, + // such as child sign-up actions and MFA completion. + useOnUserAuthenticated(children || ui.multiFactorResolver ? handleSignUp : undefined); if (ui.multiFactorResolver) { return ; @@ -57,7 +74,12 @@ export function SignUpAuthScreen({ children, onSignUp, ...props }: SignUpAuthScr {subtitleText} - + { + handleSignUp(credential.user); + }} + /> {children ? ( <> {getTranslation(ui, "messages", "dividerOr")} diff --git a/packages/react/src/hooks.test.tsx b/packages/react/src/hooks.test.tsx index 9425e1b6c..af5f4ebee 100644 --- a/packages/react/src/hooks.test.tsx +++ b/packages/react/src/hooks.test.tsx @@ -1067,33 +1067,19 @@ describe("useOnUserAuthenticated", () => { }); it("works without a callback", () => { - let authStateChangeCallback: ((user: User | null) => void) | null = null; - const mockAuth = { - onAuthStateChanged: vi.fn((callback: (user: User | null) => void) => { - authStateChangeCallback = callback; - return vi.fn(); - }), + onAuthStateChanged: vi.fn(() => vi.fn()), }; const mockUI = createMockUI({ auth: mockAuth as any, }); - const mockUser = { - uid: "test-user-id", - isAnonymous: false, - } as User; - renderHook(() => useOnUserAuthenticated(), { wrapper: ({ children }) => createFirebaseUIProvider({ children, ui: mockUI }), }); - act(() => { - authStateChangeCallback!(mockUser); - }); - - expect(mockAuth.onAuthStateChanged).toHaveBeenCalledTimes(1); + expect(mockAuth.onAuthStateChanged).not.toHaveBeenCalled(); }); it("unsubscribes from auth state changes on unmount", () => { diff --git a/packages/react/src/hooks.ts b/packages/react/src/hooks.ts index 071c7e158..69ed7359e 100644 --- a/packages/react/src/hooks.ts +++ b/packages/react/src/hooks.ts @@ -66,9 +66,13 @@ export function useOnUserAuthenticated(callback?: (user: User) => void) { const auth = ui.auth; useEffect(() => { + if (!callback) { + return; + } + return auth.onAuthStateChanged((user) => { if (user && !user.isAnonymous) { - callback?.(user); + callback(user); } }); }, [auth, callback]); diff --git a/packages/shadcn/src/components/sign-up-auth-screen.test.tsx b/packages/shadcn/src/components/sign-up-auth-screen.test.tsx index 14277d350..9726d4e8f 100644 --- a/packages/shadcn/src/components/sign-up-auth-screen.test.tsx +++ b/packages/shadcn/src/components/sign-up-auth-screen.test.tsx @@ -23,10 +23,26 @@ import { FirebaseUIProvider } from "@firebase-oss/ui-react"; import { MultiFactorResolver, type User } from "firebase/auth"; vi.mock("./sign-up-auth-form", () => ({ - SignUpAuthForm: ({ onSignUp, onSignInClick }: any) => ( + SignUpAuthForm: ({ + onSignUp, + onSignInClick, + }: { + onSignUp?: (credential: any) => void; + onSignInClick?: () => void; + }) => (
SignUpAuthForm
- {onSignUp &&
onSignUp provided
} + {onSignUp && ( + <> +
onSignUp provided
+ + + )} {onSignInClick &&
onSignInClick provided
}
), @@ -126,6 +142,39 @@ describe("", () => { expect(screen.getByTestId("onSignInClick-prop")).toBeInTheDocument(); }); + it("calls onSignUp from the resolved form credential", () => { + const onSignUp = vi.fn(); + + const mockAuth = { + onAuthStateChanged: vi.fn(() => vi.fn()), + }; + + const mockUI = createMockUI({ + auth: mockAuth as any, + locale: registerLocale("test", { + labels: { + signUp: "Register", + }, + prompts: { + enterDetailsToCreate: "Enter your details to create an account", + }, + }), + }); + + render( + + + + ); + + act(() => { + screen.getByTestId("sign-up-success-button").click(); + }); + + expect(onSignUp).toHaveBeenCalledTimes(1); + expect(onSignUp).toHaveBeenCalledWith({ uid: "signup-form-user", isAnonymous: false }); + }); + it("should not render separator when no children", () => { const mockUI = createMockUI({ locale: registerLocale("test", { @@ -279,7 +328,7 @@ describe("", () => { expect(onSignUp).toHaveBeenCalledWith(mockUser); }); - it("calls onSignUp when user authenticates via useOnUserAuthenticated hook", () => { + it("calls onSignUp when child-based sign-up flows authenticate via auth state", () => { const onSignUp = vi.fn(); let authStateChangeCallback: ((user: User | null) => void) | null = null; @@ -296,7 +345,9 @@ describe("", () => { render( - + +
Child Component
+ ); @@ -315,6 +366,44 @@ describe("", () => { expect(onSignUp).toHaveBeenCalledWith(mockUser); }); + it("dedupes the same user across form and auth state paths", () => { + const onSignUp = vi.fn(); + let authStateChangeCallback: ((user: User | null) => void) | null = null; + + const mockAuth = { + onAuthStateChanged: vi.fn((callback: (user: User | null) => void) => { + authStateChangeCallback = callback; + return vi.fn(); + }), + }; + + const mockUI = createMockUI({ + auth: mockAuth as any, + }); + + render( + + +
Child Component
+ + + ); + + act(() => { + screen.getByTestId("sign-up-success-button").click(); + }); + + act(() => { + authStateChangeCallback!({ + uid: "signup-form-user", + isAnonymous: false, + } as User); + }); + + expect(onSignUp).toHaveBeenCalledTimes(1); + expect(onSignUp).toHaveBeenCalledWith({ uid: "signup-form-user", isAnonymous: false }); + }); + it("does not call onSignUp for anonymous users", () => { const onSignUp = vi.fn(); let authStateChangeCallback: ((user: User | null) => void) | null = null; @@ -332,7 +421,9 @@ describe("", () => { render( - + +
Child Component
+ ); diff --git a/packages/shadcn/src/components/sign-up-auth-screen.tsx b/packages/shadcn/src/components/sign-up-auth-screen.tsx index acc40a126..20cd73937 100644 --- a/packages/shadcn/src/components/sign-up-auth-screen.tsx +++ b/packages/shadcn/src/components/sign-up-auth-screen.tsx @@ -18,21 +18,38 @@ import { getTranslation } from "@firebase-oss/ui-core"; import { useUI, type SignUpAuthScreenProps, useOnUserAuthenticated } from "@firebase-oss/ui-react"; +import { useCallback, useRef } from "react"; import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@/components/ui/card"; import { Separator } from "@/components/ui/separator"; import { SignUpAuthForm } from "@/components/sign-up-auth-form"; import { MultiFactorAuthAssertionScreen } from "@/components/multi-factor-auth-assertion-screen"; +import { type User } from "firebase/auth"; export type { SignUpAuthScreenProps }; export function SignUpAuthScreen({ children, onSignUp, ...props }: SignUpAuthScreenProps) { const ui = useUI(); + const handledUserIdRef = useRef(null); const titleText = getTranslation(ui, "labels", "signUp"); const subtitleText = getTranslation(ui, "prompts", "enterDetailsToCreate"); - useOnUserAuthenticated(onSignUp); + const handleSignUp = useCallback( + (user: User) => { + if (handledUserIdRef.current === user.uid) { + return; + } + + handledUserIdRef.current = user.uid; + onSignUp?.(user); + }, + [onSignUp] + ); + + // Mirror the React package behavior: the built-in form reports success from the + // resolved credential, while auth-state remains the fallback for child actions and MFA. + useOnUserAuthenticated(children || ui.multiFactorResolver ? handleSignUp : undefined); if (ui.multiFactorResolver) { return ; @@ -46,7 +63,12 @@ export function SignUpAuthScreen({ children, onSignUp, ...props }: SignUpAuthScr {subtitleText} - + { + handleSignUp(credential.user); + }} + /> {children ? ( <>