From 4e06c6489ed56e9b9459b0775e3e375ff0dde662 Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Fri, 17 Apr 2026 16:09:48 +0900 Subject: [PATCH 01/15] Fix auth router initialization stability --- .changeset/quiet-auth-stability.md | 7 ++ .../core/src/contexts/auth-context.test.tsx | 92 +++++++++++++++- packages/core/src/contexts/auth-context.tsx | 44 +++++++- packages/core/src/routing/router.test.tsx | 103 +++++++++++++++++- packages/core/src/routing/router.tsx | 59 +++++++--- 5 files changed, 278 insertions(+), 27 deletions(-) create mode 100644 .changeset/quiet-auth-stability.md diff --git a/.changeset/quiet-auth-stability.md b/.changeset/quiet-auth-stability.md new file mode 100644 index 00000000..d5b770a1 --- /dev/null +++ b/.changeset/quiet-auth-stability.md @@ -0,0 +1,7 @@ +--- +"@tailor-platform/app-shell": patch +--- + +Fix OAuth callback handling so auth redirects do not re-run unnecessarily when AppShell re-renders. + +Auth initialization now also starts from `AuthProvider`, which avoids unresolved auth state when consumers are mounted outside the router-driven AppShell flow. diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx index d558258d..1d56087b 100644 --- a/packages/core/src/contexts/auth-context.test.tsx +++ b/packages/core/src/contexts/auth-context.test.tsx @@ -38,12 +38,27 @@ describe("AuthProvider", () => { isReady: false, }; + const baseHandleCallback = overrides?.handleCallback ?? vi.fn(); + let handleCallbackInFlight: Promise | null = null; + const handleCallback = vi.fn(() => { + if (handleCallbackInFlight) { + return handleCallbackInFlight; + } + + const callbackPromise = Promise.resolve(baseHandleCallback()).finally(() => { + handleCallbackInFlight = null; + }); + handleCallbackInFlight = callbackPromise; + return callbackPromise; + }); + + const { handleCallback: _ignoredHandleCallback, ...otherOverrides } = overrides ?? {}; + return { getState: vi.fn(() => state), login: vi.fn(), logout: vi.fn(), getAuthUrl: vi.fn(), - handleCallback: vi.fn(), checkAuthStatus: vi.fn().mockResolvedValue({ isAuthenticated: false, error: null, @@ -56,7 +71,8 @@ describe("AuthProvider", () => { getAuthHeaders: vi.fn(), fetch: vi.fn(), getAppUri: vi.fn(() => "https://api.test.com"), - ...overrides, + ...otherOverrides, + handleCallback, } as EnhancedAuthClient; }; @@ -137,6 +153,33 @@ describe("AuthProvider", () => { }); describe("authentication flow", () => { + it("should initialize auth status on mount without requiring the router loader", async () => { + const state = { + isAuthenticated: false, + error: null, + isReady: false, + }; + const mockCheckAuthStatus = vi.fn().mockResolvedValue({ + isAuthenticated: true, + error: null, + isReady: true, + }); + + const mockClient = createMockAuthClient(state, { + checkAuthStatus: mockCheckAuthStatus, + }); + + render( + +
Test Content
+
, + ); + + await waitFor(() => { + expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); + }); + }); + it("should check auth status via useRootRouteContext", async () => { const state = { isAuthenticated: false, @@ -187,6 +230,51 @@ describe("AuthProvider", () => { expect(response).toBeNull(); }); + it("should deduplicate concurrent OAuth callback handling", async () => { + const state = { + isAuthenticated: true, + error: null, + isReady: true, + }; + + let resolveFirstCallback: (() => void) | undefined; + const mockHandleCallback = vi.fn(() => { + if (mockHandleCallback.mock.calls.length === 0) { + return Promise.resolve(); + } + + if (mockHandleCallback.mock.calls.length === 1) { + return new Promise((resolve) => { + resolveFirstCallback = resolve; + }); + } + + return Promise.resolve(); + }); + + const mockClient = createMockAuthClient(state, { + handleCallback: mockHandleCallback, + }); + + const { result } = renderHook(() => useRootRouteContext(), { + wrapper: ({ children }) => {children}, + }); + + expect(result.current).not.toBeNull(); + + const requestUrl = new URL("http://localhost/?code=auth-code-123&state=abc"); + const firstLoad = result.current!.loader(requestUrl); + const secondLoad = result.current!.loader(requestUrl); + + expect(mockHandleCallback).toHaveBeenCalledTimes(1); + + resolveFirstCallback?.(); + await Promise.all([firstLoad, secondLoad]); + + await result.current!.loader(requestUrl); + expect(mockHandleCallback).toHaveBeenCalledTimes(2); + }); + it("should be authenticated when logged in", async () => { const state = { isAuthenticated: true, diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx index d5719a1f..2cab31fe 100644 --- a/packages/core/src/contexts/auth-context.tsx +++ b/packages/core/src/contexts/auth-context.tsx @@ -3,6 +3,7 @@ import { useContext, useSyncExternalStore, useCallback, + useEffect, useMemo, useRef, } from "react"; @@ -288,6 +289,7 @@ const useAutoLogin = (props: { client: EnhancedAuthClient; enabled?: boolean }) */ export const AuthProvider = (props: React.PropsWithChildren) => { const client = props.client; + const initInFlightRef = useRef | null>(null); // Set up auth state subscription for auto-login orchestration const { subscribeAuthState } = useAutoLogin({ @@ -301,6 +303,38 @@ export const AuthProvider = (props: React.PropsWithChildren) // Use useSyncExternalStore for state management const authState = useSyncExternalStore(subscribeAuthState, getSnapshot); + // Start the initial auth check from the provider as well as the router loader so + // consumers outside AppShell do not get stuck in an unresolved auth state. The + // in-flight ref also coalesces overlapping checkAuthStatus() calls into one request. + const ensureInitialized = useCallback(async (): Promise => { + if (client.getState().isReady) { + return; + } + + if (initInFlightRef.current) { + return initInFlightRef.current; + } + + initInFlightRef.current = client + .checkAuthStatus() + .then(() => undefined) + .finally(() => { + initInFlightRef.current = null; + }); + + return initInFlightRef.current; + }, [client]); + + // Kick off initialization on mount instead of waiting for the first router load. + // authLoader remains as a retry path for unresolved auth state during navigation, + // but components that only use AuthProvider should not depend on RouterContainer + // being present or a navigation happening before auth becomes ready. + useEffect(() => { + ensureInitialized().catch((error) => { + console.error("Failed to check auth status:", error); + }); + }, [ensureInitialized]); + // Build the root loader inside AuthProvider so that the router layer // never needs to know about EnhancedAuthClient internals. const authLoader = useCallback( @@ -314,14 +348,14 @@ export const AuthProvider = (props: React.PropsWithChildren) } catch (error) { console.error("Failed to handle callback:", error); } + return null; } - // Only check auth status on first load (when isReady is false). - // Subsequent navigations skip this because the client already holds - // the cached auth state via useSyncExternalStore. + // Retry the initialization flow on navigation when auth state is still unresolved. + // The provider also kicks this off on mount so consumers outside AppShell do not deadlock. if (!client.getState().isReady) { try { - await client.checkAuthStatus(); + await ensureInitialized(); } catch (error) { // Intentionally swallow errors to avoid rendering the error boundary // on transient failures (e.g. network timeouts). The next navigation @@ -332,7 +366,7 @@ export const AuthProvider = (props: React.PropsWithChildren) return null; }, - [client], + [client, ensureInitialized], ); const guardComponent = props.guardComponent; diff --git a/packages/core/src/routing/router.test.tsx b/packages/core/src/routing/router.test.tsx index 0fb7c3d7..2f17da9d 100644 --- a/packages/core/src/routing/router.test.tsx +++ b/packages/core/src/routing/router.test.tsx @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { RouterContainer } from "./router"; import { AppShellConfigContext, AppShellDataContext } from "@/contexts/appshell-context"; +import * as ReactRouter from "react-router"; import { Link, Outlet, useNavigate } from "react-router"; import { defineModule, @@ -22,6 +23,7 @@ afterEach(() => { const renderWithConfig = ({ modules = [], basePath, + locale = "en", rootComponent, initialEntries, contextData = {}, @@ -31,6 +33,7 @@ const renderWithConfig = ({ }: { modules?: Array; basePath?: string; + locale?: string; rootComponent?: () => ReactNode; initialEntries: Array; contextData?: ContextData; @@ -43,7 +46,7 @@ const renderWithConfig = ({ settingsResources: [] as Array, basePath, errorBoundary: undefined, - locale: "en", + locale, }; setContextData(contextData); @@ -58,7 +61,7 @@ const renderWithConfig = ({ ); - render( + return render( authClient ? ( {tree} @@ -484,6 +487,102 @@ describe("RouterContainer with AuthProvider", () => { expect(mockHandleCallback).toHaveBeenCalled(); }); + it("does not recreate the router when auth state changes", async () => { + let snapshot = { + isAuthenticated: false, + error: null as string | null, + isReady: false, + }; + const listeners: Array<(event: { type: string }) => void> = []; + const createMemoryRouterSpy = vi.spyOn(ReactRouter, "createMemoryRouter"); + + const authClient = createMockAuthClient(snapshot, { + getState: vi.fn(() => snapshot), + addEventListener: vi.fn((listener) => { + listeners.push(listener); + return () => { + const idx = listeners.indexOf(listener); + if (idx >= 0) listeners.splice(idx, 1); + }; + }), + }); + + try { + renderWithConfig({ + modules: [], + rootComponent: () =>
Home
, + initialEntries: ["/"], + authClient, + guardComponent: () =>
Loading...
, + }); + + expect(await screen.findByText("Loading...")).toBeDefined(); + expect(createMemoryRouterSpy).toHaveBeenCalledTimes(1); + + act(() => { + snapshot = { + isAuthenticated: true, + error: null, + isReady: true, + }; + for (const listener of listeners) { + listener({ type: "auth_state_changed" }); + } + }); + + expect(await screen.findByText("Home")).toBeDefined(); + expect(createMemoryRouterSpy).toHaveBeenCalledTimes(1); + } finally { + createMemoryRouterSpy.mockRestore(); + } + }); + + it("recreates the router when route-defining config changes", async () => { + const createMemoryRouterSpy = vi.spyOn(ReactRouter, "createMemoryRouter"); + const module = defineModule({ + path: "dashboard", + component: () =>
Dashboard
, + meta: { title: "Dashboard" }, + resources: [], + }); + + try { + const view = renderWithConfig({ + modules: [module], + locale: "en", + initialEntries: ["/dashboard"], + }); + + expect(await screen.findByText("Dashboard")).toBeDefined(); + expect(createMemoryRouterSpy).toHaveBeenCalledTimes(1); + + view.rerender( + + + + + + + , + ); + + expect(await screen.findByText("Dashboard")).toBeDefined(); + expect(createMemoryRouterSpy).toHaveBeenCalledTimes(2); + } finally { + createMemoryRouterSpy.mockRestore(); + } + }); + it("calls login automatically when autoLogin is enabled and not authenticated", async () => { const mockLogin = vi.fn().mockResolvedValue(undefined); const authClient = createMockAuthClient( diff --git a/packages/core/src/routing/router.tsx b/packages/core/src/routing/router.tsx index f8bd875c..a6394400 100644 --- a/packages/core/src/routing/router.tsx +++ b/packages/core/src/routing/router.tsx @@ -1,4 +1,4 @@ -import { type PropsWithChildren, type ReactNode } from "react"; +import { type PropsWithChildren, type ReactNode, useMemo } from "react"; import { Outlet, createMemoryRouter, createBrowserRouter, RouterProvider } from "react-router"; import type { LoaderFunctionArgs, RouteObject } from "react-router"; import { createContentRoutes, RootComponentOption, wrapErrorBoundary } from "./routes"; @@ -91,25 +91,48 @@ export const RouterContainer = (props: PropsWithChildren) const { rootComponent, children } = props; const { configurations } = useAppShellConfig(); const rootRouteCtx = useRootRouteContext(); - const contentRoutes = createContentRoutes({ - modules: configurations.modules, - settingsResources: configurations.settingsResources, - rootComponent, - rootGuards: props.rootGuards, - }); - const routes = [ - createRootRoute({ configurations, rootRouteCtx, contentRoutes, children }), - ] satisfies Array; + const contentRoutes = useMemo( + () => + createContentRoutes({ + modules: configurations.modules, + settingsResources: configurations.settingsResources, + rootComponent, + rootGuards: props.rootGuards, + }), + [configurations.modules, configurations.settingsResources, rootComponent, props.rootGuards], + ); + const routes = useMemo( + () => + [ + createRootRoute({ + configurations, + rootRouteCtx, + contentRoutes, + children, + }), + ] satisfies Array, + [configurations, rootRouteCtx, contentRoutes, children], + ); const basename = configurations.basePath ? "/" + configurations.basePath : undefined; - const router = props.memory - ? createMemoryRouter(routes, { - basename, - ...(props.initialEntries ? { initialEntries: props.initialEntries } : {}), - }) - : createBrowserRouter(routes, { - basename, - }); + const initialEntries = props.memory ? props.initialEntries : undefined; + + // Keep the router instance stable across auth-driven rerenders. Recreating the + // router would re-run the root loader for the same location, which can cause + // OAuth callback URLs to be processed more than once. Still rebuild when + // route-defining inputs such as routes, basename, or initial entries change. + const router = useMemo( + () => + props.memory + ? createMemoryRouter(routes, { + basename, + ...(initialEntries ? { initialEntries } : {}), + }) + : createBrowserRouter(routes, { + basename, + }), + [basename, initialEntries, props.memory, routes], + ); return ; }; From ac4b607edbbc0b52a5b163dacc5d55dafa68eb3f Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Fri, 17 Apr 2026 16:18:37 +0900 Subject: [PATCH 02/15] Export auth initialization hook for testing --- .../core/src/contexts/auth-context.test.tsx | 178 +++++++++++++----- packages/core/src/contexts/auth-context.tsx | 87 +++++---- 2 files changed, 181 insertions(+), 84 deletions(-) diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx index 1d56087b..aed8481f 100644 --- a/packages/core/src/contexts/auth-context.test.tsx +++ b/packages/core/src/contexts/auth-context.test.tsx @@ -8,7 +8,13 @@ vi.mock("@tailor-platform/auth-public-client", () => ({ createAuthClient: vi.fn(), })); -import { AuthProvider, useAuth, useAuthSuspense, type EnhancedAuthClient } from "./auth-context"; +import { + AuthProvider, + useAuth, + useAuthInitialization, + useAuthSuspense, + type EnhancedAuthClient, +} from "./auth-context"; import { useRootRouteContext } from "./root-route-context"; afterEach(() => { @@ -45,14 +51,17 @@ describe("AuthProvider", () => { return handleCallbackInFlight; } - const callbackPromise = Promise.resolve(baseHandleCallback()).finally(() => { - handleCallbackInFlight = null; - }); + const callbackPromise = Promise.resolve(baseHandleCallback()).finally( + () => { + handleCallbackInFlight = null; + }, + ); handleCallbackInFlight = callbackPromise; return callbackPromise; }); - const { handleCallback: _ignoredHandleCallback, ...otherOverrides } = overrides ?? {}; + const { handleCallback: _ignoredHandleCallback, ...otherOverrides } = + overrides ?? {}; return { getState: vi.fn(() => state), @@ -76,6 +85,65 @@ describe("AuthProvider", () => { } as EnhancedAuthClient; }; + describe("useAuthInitialization", () => { + it("should initialize auth status on mount", async () => { + const state = { + isAuthenticated: false, + error: null, + isReady: false, + }; + const mockCheckAuthStatus = vi.fn().mockResolvedValue({ + isAuthenticated: true, + error: null, + isReady: true, + }); + + const mockClient = createMockAuthClient(state, { + checkAuthStatus: mockCheckAuthStatus, + }); + + renderHook(() => useAuthInitialization(mockClient)); + + await waitFor(() => { + expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); + }); + }); + + it("should coalesce overlapping auth initialization checks", async () => { + const state = { + isAuthenticated: false, + error: null, + isReady: false, + }; + + let resolveCheckAuthStatus: (() => void) | undefined; + const mockCheckAuthStatus = vi.fn( + () => + new Promise((resolve) => { + resolveCheckAuthStatus = resolve; + }), + ); + + const mockClient = createMockAuthClient(state, { + checkAuthStatus: mockCheckAuthStatus, + }); + + const { result } = renderHook(() => useAuthInitialization(mockClient)); + + await waitFor(() => { + expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); + }); + + const firstRetry = result.current(); + const secondRetry = result.current(); + + expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); + + resolveCheckAuthStatus?.(); + await Promise.all([firstRetry, secondRetry]); + }); + }); + describe("initial state", () => { it("should render children when not using guard component", () => { const mockClient = createMockAuthClient(); @@ -153,33 +221,6 @@ describe("AuthProvider", () => { }); describe("authentication flow", () => { - it("should initialize auth status on mount without requiring the router loader", async () => { - const state = { - isAuthenticated: false, - error: null, - isReady: false, - }; - const mockCheckAuthStatus = vi.fn().mockResolvedValue({ - isAuthenticated: true, - error: null, - isReady: true, - }); - - const mockClient = createMockAuthClient(state, { - checkAuthStatus: mockCheckAuthStatus, - }); - - render( - -
Test Content
-
, - ); - - await waitFor(() => { - expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); - }); - }); - it("should check auth status via useRootRouteContext", async () => { const state = { isAuthenticated: false, @@ -197,11 +238,15 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useRootRouteContext(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); expect(result.current).not.toBeNull(); - const response = await result.current!.loader(new URL("http://localhost/")); + const response = await result.current!.loader( + new URL("http://localhost/"), + ); expect(mockCheckAuthStatus).toHaveBeenCalled(); expect(response).toBeNull(); }); @@ -219,7 +264,9 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useRootRouteContext(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); expect(result.current).not.toBeNull(); @@ -257,12 +304,16 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useRootRouteContext(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); expect(result.current).not.toBeNull(); - const requestUrl = new URL("http://localhost/?code=auth-code-123&state=abc"); + const requestUrl = new URL( + "http://localhost/?code=auth-code-123&state=abc", + ); const firstLoad = result.current!.loader(requestUrl); const secondLoad = result.current!.loader(requestUrl); @@ -290,7 +341,9 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); await waitFor(() => { @@ -316,7 +369,9 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); await waitFor(() => { @@ -344,7 +399,9 @@ describe("AuthProvider", () => { const mockClient = createMockAuthClient(state); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); await waitFor(() => { @@ -371,7 +428,9 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); await waitFor(() => { @@ -399,7 +458,9 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); await waitFor(() => { @@ -459,7 +520,9 @@ describe("AuthProvider", () => { }); it("should resolve suspense when auth becomes ready", async () => { - let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; + let authEventListener: + | ((event: { type: string; data?: unknown }) => void) + | undefined; const mockAddEventListener = vi.fn( (listener: (event: { type: string; data?: unknown }) => void) => { @@ -486,7 +549,12 @@ describe("AuthProvider", () => { const TestComponent = () => { const { isAuthenticated } = useAuthSuspense(); - return
Content Loaded: {isAuthenticated ? "authenticated" : "not authenticated"}
; + return ( +
+ Content Loaded:{" "} + {isAuthenticated ? "authenticated" : "not authenticated"} +
+ ); }; render( @@ -530,7 +598,11 @@ describe("AuthProvider", () => { const TestComponent = () => { const { isAuthenticated } = useAuthSuspense(); - return
Loaded: {isAuthenticated ? "authenticated" : "unauthenticated"}
; + return ( +
+ Loaded: {isAuthenticated ? "authenticated" : "unauthenticated"} +
+ ); }; render( @@ -584,7 +656,9 @@ describe("AuthProvider", () => { }); it("should login when auth state changes to unauthenticated", async () => { - let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; + let authEventListener: + | ((event: { type: string; data?: unknown }) => void) + | undefined; const mockAddEventListener = vi.fn( (listener: (event: { type: string; data?: unknown }) => void) => { @@ -689,7 +763,9 @@ describe("AuthProvider", () => { describe("event listeners", () => { it("should listen to auth state changes via useSyncExternalStore", async () => { - let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; + let authEventListener: + | ((event: { type: string; data?: unknown }) => void) + | undefined; const mockAddEventListener = vi.fn( (listener: (event: { type: string; data?: unknown }) => void) => { @@ -713,7 +789,9 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); await waitFor(() => { @@ -756,7 +834,9 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); await waitFor(() => { diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx index 2cab31fe..3354eff2 100644 --- a/packages/core/src/contexts/auth-context.tsx +++ b/packages/core/src/contexts/auth-context.tsx @@ -12,6 +12,7 @@ import { type AuthClient, } from "@tailor-platform/auth-public-client"; import { RootRouteContext } from "@/contexts/root-route-context"; +import { A } from "node_modules/react-router/dist/development/context-phCt_zmH.mjs"; // ============================================================================ // Auth Client @@ -212,7 +213,10 @@ type AuthProviderProps = { * - initial deferred auto-login attempt * - duplicate login prevention */ -const useAutoLogin = (props: { client: EnhancedAuthClient; enabled?: boolean }) => { +const useAutoLogin = (props: { + client: EnhancedAuthClient; + enabled?: boolean; +}) => { // Prevent duplicate login redirects when multiple auth_state_changed // events fire before the first login attempt settles. const loginInFlightRef = useRef | null>(null); @@ -263,6 +267,44 @@ const useAutoLogin = (props: { client: EnhancedAuthClient; enabled?: boolean }) }; }; +/** + * Starts auth initialization on mount and coalesces overlapping readiness checks. + * Returns a function that callers can reuse as a retry path when auth is still unresolved. + */ +export const useAuthInitialization = (client: EnhancedAuthClient) => { + const initInFlightRef = useRef | null>(null); + + const ensureInitialized = useCallback(async (): Promise => { + if (client.getState().isReady) { + return; + } + + if (initInFlightRef.current) { + return initInFlightRef.current; + } + + initInFlightRef.current = client + .checkAuthStatus() + .then(() => undefined) + .finally(() => { + initInFlightRef.current = null; + }); + + return initInFlightRef.current; + }, [client]); + + // Kick off initialization on mount instead of waiting for the first router load. + // Router-driven loads still reuse the returned function as a retry path when + // auth remains unresolved, but AuthProvider should not depend on navigation. + useEffect(() => { + ensureInitialized().catch((error) => { + console.error("Failed to check auth status:", error); + }); + }, [ensureInitialized]); + + return ensureInitialized; +}; + /** * Authentication provider component. * @@ -287,9 +329,10 @@ const useAutoLogin = (props: { client: EnhancedAuthClient; enabled?: boolean }) * } * ``` */ -export const AuthProvider = (props: React.PropsWithChildren) => { +export const AuthProvider = ( + props: React.PropsWithChildren, +) => { const client = props.client; - const initInFlightRef = useRef | null>(null); // Set up auth state subscription for auto-login orchestration const { subscribeAuthState } = useAutoLogin({ @@ -303,37 +346,9 @@ export const AuthProvider = (props: React.PropsWithChildren) // Use useSyncExternalStore for state management const authState = useSyncExternalStore(subscribeAuthState, getSnapshot); - // Start the initial auth check from the provider as well as the router loader so - // consumers outside AppShell do not get stuck in an unresolved auth state. The - // in-flight ref also coalesces overlapping checkAuthStatus() calls into one request. - const ensureInitialized = useCallback(async (): Promise => { - if (client.getState().isReady) { - return; - } - - if (initInFlightRef.current) { - return initInFlightRef.current; - } - - initInFlightRef.current = client - .checkAuthStatus() - .then(() => undefined) - .finally(() => { - initInFlightRef.current = null; - }); - - return initInFlightRef.current; - }, [client]); - - // Kick off initialization on mount instead of waiting for the first router load. - // authLoader remains as a retry path for unresolved auth state during navigation, - // but components that only use AuthProvider should not depend on RouterContainer - // being present or a navigation happening before auth becomes ready. - useEffect(() => { - ensureInitialized().catch((error) => { - console.error("Failed to check auth status:", error); - }); - }, [ensureInitialized]); + // Reuse the same initialization path for mount-time bootstrapping and + // loader-time retries so unresolved auth state is handled consistently. + const ensureInitialized = useAuthInitialization(client); // Build the root loader inside AuthProvider so that the router layer // never needs to know about EnhancedAuthClient internals. @@ -408,7 +423,9 @@ export const AuthProvider = (props: React.PropsWithChildren) const useAuthContext = () => { const context = useContext(AuthContext); if (!context) { - throw new Error("useAuth/useAuthSuspense must be used within an AuthProvider"); + throw new Error( + "useAuth/useAuthSuspense must be used within an AuthProvider", + ); } return context; }; From bf18f52bf50d52e98c1730ba0a92fabc1c537cc3 Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Fri, 17 Apr 2026 16:19:28 +0900 Subject: [PATCH 03/15] Format --- .../core/src/contexts/auth-context.test.tsx | 84 +++++-------------- packages/core/src/contexts/auth-context.tsx | 13 +-- 2 files changed, 24 insertions(+), 73 deletions(-) diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx index aed8481f..3e31f40e 100644 --- a/packages/core/src/contexts/auth-context.test.tsx +++ b/packages/core/src/contexts/auth-context.test.tsx @@ -51,17 +51,14 @@ describe("AuthProvider", () => { return handleCallbackInFlight; } - const callbackPromise = Promise.resolve(baseHandleCallback()).finally( - () => { - handleCallbackInFlight = null; - }, - ); + const callbackPromise = Promise.resolve(baseHandleCallback()).finally(() => { + handleCallbackInFlight = null; + }); handleCallbackInFlight = callbackPromise; return callbackPromise; }); - const { handleCallback: _ignoredHandleCallback, ...otherOverrides } = - overrides ?? {}; + const { handleCallback: _ignoredHandleCallback, ...otherOverrides } = overrides ?? {}; return { getState: vi.fn(() => state), @@ -238,15 +235,11 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useRootRouteContext(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); expect(result.current).not.toBeNull(); - const response = await result.current!.loader( - new URL("http://localhost/"), - ); + const response = await result.current!.loader(new URL("http://localhost/")); expect(mockCheckAuthStatus).toHaveBeenCalled(); expect(response).toBeNull(); }); @@ -264,9 +257,7 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useRootRouteContext(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); expect(result.current).not.toBeNull(); @@ -304,16 +295,12 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useRootRouteContext(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); expect(result.current).not.toBeNull(); - const requestUrl = new URL( - "http://localhost/?code=auth-code-123&state=abc", - ); + const requestUrl = new URL("http://localhost/?code=auth-code-123&state=abc"); const firstLoad = result.current!.loader(requestUrl); const secondLoad = result.current!.loader(requestUrl); @@ -341,9 +328,7 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); await waitFor(() => { @@ -369,9 +354,7 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); await waitFor(() => { @@ -399,9 +382,7 @@ describe("AuthProvider", () => { const mockClient = createMockAuthClient(state); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); await waitFor(() => { @@ -428,9 +409,7 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); await waitFor(() => { @@ -458,9 +437,7 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); await waitFor(() => { @@ -520,9 +497,7 @@ describe("AuthProvider", () => { }); it("should resolve suspense when auth becomes ready", async () => { - let authEventListener: - | ((event: { type: string; data?: unknown }) => void) - | undefined; + let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; const mockAddEventListener = vi.fn( (listener: (event: { type: string; data?: unknown }) => void) => { @@ -549,12 +524,7 @@ describe("AuthProvider", () => { const TestComponent = () => { const { isAuthenticated } = useAuthSuspense(); - return ( -
- Content Loaded:{" "} - {isAuthenticated ? "authenticated" : "not authenticated"} -
- ); + return
Content Loaded: {isAuthenticated ? "authenticated" : "not authenticated"}
; }; render( @@ -598,11 +568,7 @@ describe("AuthProvider", () => { const TestComponent = () => { const { isAuthenticated } = useAuthSuspense(); - return ( -
- Loaded: {isAuthenticated ? "authenticated" : "unauthenticated"} -
- ); + return
Loaded: {isAuthenticated ? "authenticated" : "unauthenticated"}
; }; render( @@ -656,9 +622,7 @@ describe("AuthProvider", () => { }); it("should login when auth state changes to unauthenticated", async () => { - let authEventListener: - | ((event: { type: string; data?: unknown }) => void) - | undefined; + let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; const mockAddEventListener = vi.fn( (listener: (event: { type: string; data?: unknown }) => void) => { @@ -763,9 +727,7 @@ describe("AuthProvider", () => { describe("event listeners", () => { it("should listen to auth state changes via useSyncExternalStore", async () => { - let authEventListener: - | ((event: { type: string; data?: unknown }) => void) - | undefined; + let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; const mockAddEventListener = vi.fn( (listener: (event: { type: string; data?: unknown }) => void) => { @@ -789,9 +751,7 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); await waitFor(() => { @@ -834,9 +794,7 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); await waitFor(() => { diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx index 3354eff2..7623b1b3 100644 --- a/packages/core/src/contexts/auth-context.tsx +++ b/packages/core/src/contexts/auth-context.tsx @@ -213,10 +213,7 @@ type AuthProviderProps = { * - initial deferred auto-login attempt * - duplicate login prevention */ -const useAutoLogin = (props: { - client: EnhancedAuthClient; - enabled?: boolean; -}) => { +const useAutoLogin = (props: { client: EnhancedAuthClient; enabled?: boolean }) => { // Prevent duplicate login redirects when multiple auth_state_changed // events fire before the first login attempt settles. const loginInFlightRef = useRef | null>(null); @@ -329,9 +326,7 @@ export const useAuthInitialization = (client: EnhancedAuthClient) => { * } * ``` */ -export const AuthProvider = ( - props: React.PropsWithChildren, -) => { +export const AuthProvider = (props: React.PropsWithChildren) => { const client = props.client; // Set up auth state subscription for auto-login orchestration @@ -423,9 +418,7 @@ export const AuthProvider = ( const useAuthContext = () => { const context = useContext(AuthContext); if (!context) { - throw new Error( - "useAuth/useAuthSuspense must be used within an AuthProvider", - ); + throw new Error("useAuth/useAuthSuspense must be used within an AuthProvider"); } return context; }; From 7b8dba1b664e51a1cf633a861df65cee95492236 Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Fri, 17 Apr 2026 16:20:02 +0900 Subject: [PATCH 04/15] Remove unused import --- packages/core/src/contexts/auth-context.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx index 7623b1b3..eb959367 100644 --- a/packages/core/src/contexts/auth-context.tsx +++ b/packages/core/src/contexts/auth-context.tsx @@ -12,7 +12,6 @@ import { type AuthClient, } from "@tailor-platform/auth-public-client"; import { RootRouteContext } from "@/contexts/root-route-context"; -import { A } from "node_modules/react-router/dist/development/context-phCt_zmH.mjs"; // ============================================================================ // Auth Client From 721806b2aec79eb06a751ecc493d3d61a774dc05 Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Fri, 17 Apr 2026 16:26:31 +0900 Subject: [PATCH 05/15] Fix type errors --- packages/core/src/contexts/auth-context.test.tsx | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx index 3e31f40e..a4aad55f 100644 --- a/packages/core/src/contexts/auth-context.test.tsx +++ b/packages/core/src/contexts/auth-context.test.tsx @@ -116,8 +116,17 @@ describe("AuthProvider", () => { let resolveCheckAuthStatus: (() => void) | undefined; const mockCheckAuthStatus = vi.fn( () => - new Promise((resolve) => { - resolveCheckAuthStatus = resolve; + new Promise<{ + isAuthenticated: boolean; + error: null; + isReady: true; + }>((resolve) => { + resolveCheckAuthStatus = () => + resolve({ + isAuthenticated: true, + error: null, + isReady: true, + }); }), ); From 3ae83fd42a672116e736d7b0dc1ca9a1e2f25dcf Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Fri, 17 Apr 2026 16:41:16 +0900 Subject: [PATCH 06/15] Fix auth callback auto-login race --- .../core/src/contexts/auth-context.test.tsx | 81 +++++++++++++++++++ packages/core/src/contexts/auth-context.tsx | 16 +++- 2 files changed, 95 insertions(+), 2 deletions(-) diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx index a4aad55f..17ede700 100644 --- a/packages/core/src/contexts/auth-context.test.tsx +++ b/packages/core/src/contexts/auth-context.test.tsx @@ -21,6 +21,7 @@ afterEach(() => { cleanup(); vi.clearAllMocks(); vi.unstubAllGlobals(); + window.history.replaceState({}, "", "/"); }); const LoadingGuard = () =>
Loading...
; @@ -148,6 +149,33 @@ describe("AuthProvider", () => { resolveCheckAuthStatus?.(); await Promise.all([firstRetry, secondRetry]); }); + + it("should skip auth initialization while handling an OAuth callback", async () => { + window.history.replaceState({}, "", "/?code=auth-code-123&state=abc"); + + const state = { + isAuthenticated: false, + error: null, + isReady: false, + }; + const mockCheckAuthStatus = vi.fn().mockResolvedValue({ + isAuthenticated: false, + error: null, + isReady: true, + }); + + const mockClient = createMockAuthClient(state, { + checkAuthStatus: mockCheckAuthStatus, + }); + + renderHook(() => useAuthInitialization(mockClient)); + + await act(async () => { + await Promise.resolve(); + }); + + expect(mockCheckAuthStatus).not.toHaveBeenCalled(); + }); }); describe("initial state", () => { @@ -732,6 +760,59 @@ describe("AuthProvider", () => { { timeout: 1000 }, ); }); + + it("should not login while the current URL is an OAuth callback", async () => { + window.history.replaceState({}, "", "/?code=auth-code-123&state=abc"); + + let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; + + const mockAddEventListener = vi.fn( + (listener: (event: { type: string; data?: unknown }) => void) => { + authEventListener = listener; + return () => {}; + }, + ); + + let currentState = { + isAuthenticated: false, + error: null as string | null, + isReady: true, + }; + + const mockLogin = vi.fn().mockResolvedValue(undefined); + const mockClient = createMockAuthClient(undefined, { + login: mockLogin, + addEventListener: mockAddEventListener, + getState: vi.fn(() => currentState), + }); + + render( + +
Content
+
, + ); + + await act(async () => { + await Promise.resolve(); + }); + + expect(mockLogin).not.toHaveBeenCalled(); + + act(() => { + currentState = { + isAuthenticated: false, + error: null, + isReady: true, + }; + authEventListener?.({ type: "auth_state_changed", data: {} }); + }); + + await act(async () => { + await Promise.resolve(); + }); + + expect(mockLogin).not.toHaveBeenCalled(); + }); }); describe("event listeners", () => { diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx index eb959367..430a7740 100644 --- a/packages/core/src/contexts/auth-context.tsx +++ b/packages/core/src/contexts/auth-context.tsx @@ -161,6 +161,17 @@ type AuthContextType = { const AuthContext = createContext(null); +const isOAuthCallbackUrl = (url: URL) => + url.searchParams.has("code") || url.searchParams.has("error"); + +const isCurrentOAuthCallbackUrl = () => { + if (typeof window === "undefined") { + return false; + } + + return isOAuthCallbackUrl(new URL(window.location.href)); +}; + /** * Guard component that shows a fallback UI while auth is not ready or * not authenticated. Defined here so that the router layer does not @@ -222,6 +233,7 @@ const useAutoLogin = (props: { client: EnhancedAuthClient; enabled?: boolean }) const authState = props.client.getState(); if ( !props.enabled || + isCurrentOAuthCallbackUrl() || !authState.isReady || authState.isAuthenticated || loginInFlightRef.current @@ -271,7 +283,7 @@ export const useAuthInitialization = (client: EnhancedAuthClient) => { const initInFlightRef = useRef | null>(null); const ensureInitialized = useCallback(async (): Promise => { - if (client.getState().isReady) { + if (isCurrentOAuthCallbackUrl() || client.getState().isReady) { return; } @@ -351,7 +363,7 @@ export const AuthProvider = (props: React.PropsWithChildren) // The "code" query parameter indicates a redirect back from the OAuth provider. // handleCallback() internally cleans up the OAuth-related query parameters // from the URL, so no additional URL cleanup is needed here. - if (requestUrl.searchParams.has("code")) { + if (isOAuthCallbackUrl(requestUrl)) { try { await client.handleCallback(); } catch (error) { From f1a128af4bce32d83a90e2eafa63ab0b2ad5529b Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Fri, 17 Apr 2026 16:53:38 +0900 Subject: [PATCH 07/15] Refine auth initialization responsibilities --- .../core/src/contexts/auth-context.test.tsx | 30 ++++++---- packages/core/src/contexts/auth-context.tsx | 55 +++++++------------ .../core/src/contexts/root-route-context.tsx | 3 +- packages/core/src/routing/router.test.tsx | 8 +-- packages/core/src/routing/router.tsx | 2 +- 5 files changed, 46 insertions(+), 52 deletions(-) diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx index 17ede700..8e94f64d 100644 --- a/packages/core/src/contexts/auth-context.test.tsx +++ b/packages/core/src/contexts/auth-context.test.tsx @@ -11,7 +11,7 @@ vi.mock("@tailor-platform/auth-public-client", () => ({ import { AuthProvider, useAuth, - useAuthInitialization, + useEnsureAuthInitialized, useAuthSuspense, type EnhancedAuthClient, } from "./auth-context"; @@ -83,7 +83,7 @@ describe("AuthProvider", () => { } as EnhancedAuthClient; }; - describe("useAuthInitialization", () => { + describe("useEnsureAuthInitialized", () => { it("should initialize auth status on mount", async () => { const state = { isAuthenticated: false, @@ -100,11 +100,13 @@ describe("AuthProvider", () => { checkAuthStatus: mockCheckAuthStatus, }); - renderHook(() => useAuthInitialization(mockClient)); + const { result } = renderHook(() => useEnsureAuthInitialized(mockClient)); - await waitFor(() => { - expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); + await act(async () => { + await result.current(); }); + + expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); }); it("should coalesce overlapping auth initialization checks", async () => { @@ -135,7 +137,9 @@ describe("AuthProvider", () => { checkAuthStatus: mockCheckAuthStatus, }); - const { result } = renderHook(() => useAuthInitialization(mockClient)); + const { result } = renderHook(() => useEnsureAuthInitialized(mockClient)); + + const mountRetry = result.current(); await waitFor(() => { expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); @@ -147,7 +151,7 @@ describe("AuthProvider", () => { expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); resolveCheckAuthStatus?.(); - await Promise.all([firstRetry, secondRetry]); + await Promise.all([mountRetry, firstRetry, secondRetry]); }); it("should skip auth initialization while handling an OAuth callback", async () => { @@ -168,10 +172,10 @@ describe("AuthProvider", () => { checkAuthStatus: mockCheckAuthStatus, }); - renderHook(() => useAuthInitialization(mockClient)); + const { result } = renderHook(() => useEnsureAuthInitialized(mockClient)); await act(async () => { - await Promise.resolve(); + await result.current(); }); expect(mockCheckAuthStatus).not.toHaveBeenCalled(); @@ -255,7 +259,7 @@ describe("AuthProvider", () => { }); describe("authentication flow", () => { - it("should check auth status via useRootRouteContext", async () => { + it("should not check auth status via useRootRouteContext on non-callback URLs", async () => { const state = { isAuthenticated: false, error: null, @@ -276,8 +280,12 @@ describe("AuthProvider", () => { }); expect(result.current).not.toBeNull(); + await waitFor(() => { + expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); + }); + const response = await result.current!.loader(new URL("http://localhost/")); - expect(mockCheckAuthStatus).toHaveBeenCalled(); + expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); expect(response).toBeNull(); }); diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx index 430a7740..4e603b14 100644 --- a/packages/core/src/contexts/auth-context.tsx +++ b/packages/core/src/contexts/auth-context.tsx @@ -276,10 +276,13 @@ const useAutoLogin = (props: { client: EnhancedAuthClient; enabled?: boolean }) }; /** - * Starts auth initialization on mount and coalesces overlapping readiness checks. - * Returns a function that callers can reuse as a retry path when auth is still unresolved. + * Builds a stable function that resolves the initial auth state once. + * + * AuthProvider uses the returned function from its own useEffect so the + * initialization flow stays visible at the call site, while overlapping + * checks still collapse into a single request. */ -export const useAuthInitialization = (client: EnhancedAuthClient) => { +export const useEnsureAuthInitialized = (client: EnhancedAuthClient) => { const initInFlightRef = useRef | null>(null); const ensureInitialized = useCallback(async (): Promise => { @@ -301,15 +304,6 @@ export const useAuthInitialization = (client: EnhancedAuthClient) => { return initInFlightRef.current; }, [client]); - // Kick off initialization on mount instead of waiting for the first router load. - // Router-driven loads still reuse the returned function as a retry path when - // auth remains unresolved, but AuthProvider should not depend on navigation. - useEffect(() => { - ensureInitialized().catch((error) => { - console.error("Failed to check auth status:", error); - }); - }, [ensureInitialized]); - return ensureInitialized; }; @@ -352,17 +346,23 @@ export const AuthProvider = (props: React.PropsWithChildren) // Use useSyncExternalStore for state management const authState = useSyncExternalStore(subscribeAuthState, getSnapshot); - // Reuse the same initialization path for mount-time bootstrapping and - // loader-time retries so unresolved auth state is handled consistently. - const ensureInitialized = useAuthInitialization(client); + // Prepare a shared initialization function so AuthProvider can start the + // first auth check itself without depending on router navigation. + const ensureAuthInitialized = useEnsureAuthInitialized(client); + + useEffect(() => { + ensureAuthInitialized().catch((error) => { + console.error("Failed to check auth status:", error); + }); + }, [ensureAuthInitialized]); - // Build the root loader inside AuthProvider so that the router layer - // never needs to know about EnhancedAuthClient internals. + // When the auth server redirects back with OAuth parameters, resolve that + // callback before the route tree continues so the app renders from the + // post-login URL and final auth state. const authLoader = useCallback( async (requestUrl: URL): Promise => { - // The "code" query parameter indicates a redirect back from the OAuth provider. - // handleCallback() internally cleans up the OAuth-related query parameters - // from the URL, so no additional URL cleanup is needed here. + // handleCallback() exchanges the callback parameters, stores the new + // session, and removes the temporary OAuth query parameters from the URL. if (isOAuthCallbackUrl(requestUrl)) { try { await client.handleCallback(); @@ -372,22 +372,9 @@ export const AuthProvider = (props: React.PropsWithChildren) return null; } - // Retry the initialization flow on navigation when auth state is still unresolved. - // The provider also kicks this off on mount so consumers outside AppShell do not deadlock. - if (!client.getState().isReady) { - try { - await ensureInitialized(); - } catch (error) { - // Intentionally swallow errors to avoid rendering the error boundary - // on transient failures (e.g. network timeouts). The next navigation - // will re-run this loader and retry automatically. - console.error("Failed to check auth status:", error); - } - } - return null; }, - [client, ensureInitialized], + [client], ); const guardComponent = props.guardComponent; diff --git a/packages/core/src/contexts/root-route-context.tsx b/packages/core/src/contexts/root-route-context.tsx index 3e943309..d0aa8f6f 100644 --- a/packages/core/src/contexts/root-route-context.tsx +++ b/packages/core/src/contexts/root-route-context.tsx @@ -7,8 +7,7 @@ export type RootRouteLoaderFn = (requestUrl: URL) => Promise; export type RootRouteContextType = { /** * Runs in the react-router loader phase (before rendering). - * Used for side effects such as OAuth callback handling and auth - * status checks. + * Used for side effects such as OAuth callback handling. * * ``` * loader runs ──▶ rendering starts ──▶ wrapComponent applied diff --git a/packages/core/src/routing/router.test.tsx b/packages/core/src/routing/router.test.tsx index 2f17da9d..7375b694 100644 --- a/packages/core/src/routing/router.test.tsx +++ b/packages/core/src/routing/router.test.tsx @@ -433,7 +433,7 @@ const createMockAuthClient = ( }; describe("RouterContainer with AuthProvider", () => { - it("calls checkAuthStatus via loader on initial load", async () => { + it("does not call checkAuthStatus via loader on initial load", async () => { const mockCheckAuthStatus = vi.fn().mockResolvedValue({ isAuthenticated: true, error: null, @@ -458,7 +458,7 @@ describe("RouterContainer with AuthProvider", () => { }); await screen.findByText("Dashboard"); - expect(mockCheckAuthStatus).toHaveBeenCalled(); + expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); }); it("calls handleCallback when OAuth code is present in URL", async () => { @@ -694,8 +694,8 @@ describe("RouterContainer with AuthProvider", () => { it("transitions from guard to children when auth state changes", async () => { // Mutable snapshot; initially not ready, not authenticated. - // The loader calls checkAuthStatus, but the mock does NOT update the - // snapshot during the loader — so the guard is shown on first render. + // Mount-time initialization runs outside the router loader, so the guard + // remains visible until the auth client publishes a ready state. let snapshot = { isAuthenticated: false, error: null as string | null, diff --git a/packages/core/src/routing/router.tsx b/packages/core/src/routing/router.tsx index a6394400..d5025684 100644 --- a/packages/core/src/routing/router.tsx +++ b/packages/core/src/routing/router.tsx @@ -16,7 +16,7 @@ import { useRootRouteContext, type RootRouteContextType } from "@/contexts/root- * error boundary, and optional element wrapping into a single RouteObject. * * When AuthProvider wraps AppShell, rootRouteCtx provides: - * - loader: runs before rendering (e.g. OAuth callback, auth status check) + * - loader: runs before rendering (e.g. OAuth callback handling) * - wrapComponent: wraps the root component (e.g. guard UI while loading) * When AuthProvider is not used, rootRouteCtx is null and these are skipped. */ From 62dfb229ff0ce8bd7428decb133e716243aeccbe Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Fri, 17 Apr 2026 16:57:21 +0900 Subject: [PATCH 08/15] Add code comment --- packages/core/src/contexts/auth-context.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx index 4e603b14..aab8af36 100644 --- a/packages/core/src/contexts/auth-context.tsx +++ b/packages/core/src/contexts/auth-context.tsx @@ -350,15 +350,18 @@ export const AuthProvider = (props: React.PropsWithChildren) // first auth check itself without depending on router navigation. const ensureAuthInitialized = useEnsureAuthInitialized(client); + // AuthProvider owns the normal startup path: on mount, ask the auth client + // to resolve the current session so consumers can rely on authState even + // before any router loader has run. useEffect(() => { ensureAuthInitialized().catch((error) => { console.error("Failed to check auth status:", error); }); }, [ensureAuthInitialized]); - // When the auth server redirects back with OAuth parameters, resolve that - // callback before the route tree continues so the app renders from the - // post-login URL and final auth state. + // The router loader is reserved for OAuth callback URLs. Its job is to + // finish the redirect handshake before rendering continues, while the + // ordinary "am I already signed in?" check stays with AuthProvider. const authLoader = useCallback( async (requestUrl: URL): Promise => { // handleCallback() exchanges the callback parameters, stores the new From 763d1ce7040b2983f04f8063f44d270738f38d1c Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Sun, 19 Apr 2026 17:50:05 +0900 Subject: [PATCH 09/15] fix(auth): resolve stuck unauthenticated state when guard blocks AppShell --- .changeset/quiet-auth-stability.md | 4 +- .../core/src/contexts/auth-context.test.tsx | 124 ++++++----------- packages/core/src/contexts/auth-context.tsx | 125 +++++++++++------- .../core/src/contexts/root-route-context.tsx | 54 -------- packages/core/src/routing/router.test.tsx | 65 +++++---- packages/core/src/routing/router.tsx | 37 ++---- 6 files changed, 161 insertions(+), 248 deletions(-) delete mode 100644 packages/core/src/contexts/root-route-context.tsx diff --git a/.changeset/quiet-auth-stability.md b/.changeset/quiet-auth-stability.md index d5b770a1..f6b9853d 100644 --- a/.changeset/quiet-auth-stability.md +++ b/.changeset/quiet-auth-stability.md @@ -2,6 +2,4 @@ "@tailor-platform/app-shell": patch --- -Fix OAuth callback handling so auth redirects do not re-run unnecessarily when AppShell re-renders. - -Auth initialization now also starts from `AuthProvider`, which avoids unresolved auth state when consumers are mounted outside the router-driven AppShell flow. +Fixed a bug where placing a guard component between `AuthProvider` and `AppShell` could block `AppShell` from mounting, preventing the authentication flow from ever starting and leaving the app stuck in an unauthenticated state. diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx index 8e94f64d..f9611872 100644 --- a/packages/core/src/contexts/auth-context.test.tsx +++ b/packages/core/src/contexts/auth-context.test.tsx @@ -15,7 +15,6 @@ import { useAuthSuspense, type EnhancedAuthClient, } from "./auth-context"; -import { useRootRouteContext } from "./root-route-context"; afterEach(() => { cleanup(); @@ -78,6 +77,7 @@ describe("AuthProvider", () => { getAuthHeaders: vi.fn(), fetch: vi.fn(), getAppUri: vi.fn(() => "https://api.test.com"), + callbackPromise: null, ...otherOverrides, handleCallback, } as EnhancedAuthClient; @@ -195,7 +195,7 @@ describe("AuthProvider", () => { expect(screen.getByText("Test Content")).toBeDefined(); }); - it("should always render children even with guardComponent when not ready", () => { + it("should show guard component when not ready", () => { const state = { isAuthenticated: false, error: null, @@ -209,11 +209,12 @@ describe("AuthProvider", () => {
, ); - // AuthProvider always renders children; guard rendering is handled by the router - expect(screen.getByText("Protected Content")).toBeDefined(); + // Guard is now rendered directly by AuthProvider + expect(screen.getByText("Loading...")).toBeDefined(); + expect(screen.queryByText("Protected Content")).toBeNull(); }); - it("should always render children even with guardComponent when not authenticated", async () => { + it("should show guard component when not authenticated", async () => { const state = { isAuthenticated: false, error: null, @@ -227,8 +228,9 @@ describe("AuthProvider", () => { , ); - // AuthProvider always renders children; guard rendering is handled by the router - expect(screen.getByText("Protected Content")).toBeDefined(); + // Guard is now rendered directly by AuthProvider + expect(screen.getByText("Please log in")).toBeDefined(); + expect(screen.queryByText("Protected Content")).toBeNull(); }); it("should show children when authenticated", async () => { @@ -259,7 +261,7 @@ describe("AuthProvider", () => { }); describe("authentication flow", () => { - it("should not check auth status via useRootRouteContext on non-callback URLs", async () => { + it("should call checkAuthStatus once on mount for non-callback URLs", async () => { const state = { isAuthenticated: false, error: null, @@ -275,87 +277,43 @@ describe("AuthProvider", () => { checkAuthStatus: mockCheckAuthStatus, }); - const { result } = renderHook(() => useRootRouteContext(), { - wrapper: ({ children }) => {children}, - }); + render( + +
Content
+
, + ); - expect(result.current).not.toBeNull(); await waitFor(() => { expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); }); - - const response = await result.current!.loader(new URL("http://localhost/")); - expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); - expect(response).toBeNull(); }); - it("should handle OAuth callback via useRootRouteContext when code is present", async () => { - const state = { - isAuthenticated: true, - error: null, - isReady: true, - }; + it("should suspend children until callbackPromise resolves when set on client", async () => { const mockHandleCallback = vi.fn().mockResolvedValue(undefined); - - const mockClient = createMockAuthClient(state, { - handleCallback: mockHandleCallback, - }); - - const { result } = renderHook(() => useRootRouteContext(), { - wrapper: ({ children }) => {children}, - }); - - expect(result.current).not.toBeNull(); - const response = await result.current!.loader( - new URL("http://localhost/?code=auth-code-123"), - ); - expect(mockHandleCallback).toHaveBeenCalled(); - expect(response).toBeNull(); - }); - - it("should deduplicate concurrent OAuth callback handling", async () => { + // Simulate what createAuthClient does when ?code= is in the URL: + // kick off handleCallback() and store the promise on the client. + const callbackPromise = Promise.resolve().then(() => mockHandleCallback()); const state = { isAuthenticated: true, error: null, isReady: true, }; - - let resolveFirstCallback: (() => void) | undefined; - const mockHandleCallback = vi.fn(() => { - if (mockHandleCallback.mock.calls.length === 0) { - return Promise.resolve(); - } - - if (mockHandleCallback.mock.calls.length === 1) { - return new Promise((resolve) => { - resolveFirstCallback = resolve; - }); - } - - return Promise.resolve(); - }); - const mockClient = createMockAuthClient(state, { handleCallback: mockHandleCallback, + callbackPromise, }); - const { result } = renderHook(() => useRootRouteContext(), { - wrapper: ({ children }) => {children}, + await act(async () => { + render( + +
Content
+
, + ); + await callbackPromise; }); - expect(result.current).not.toBeNull(); - - const requestUrl = new URL("http://localhost/?code=auth-code-123&state=abc"); - const firstLoad = result.current!.loader(requestUrl); - const secondLoad = result.current!.loader(requestUrl); - - expect(mockHandleCallback).toHaveBeenCalledTimes(1); - - resolveFirstCallback?.(); - await Promise.all([firstLoad, secondLoad]); - - await result.current!.loader(requestUrl); - expect(mockHandleCallback).toHaveBeenCalledTimes(2); + expect(screen.getByText("Content")).toBeDefined(); + expect(mockHandleCallback).toHaveBeenCalled(); }); it("should be authenticated when logged in", async () => { @@ -632,7 +590,7 @@ describe("AuthProvider", () => { }); describe("autoLogin", () => { - it("should not call login from the loader (autoLogin is handled by provider subscription)", async () => { + it("should not call login on initial render when autoLogin is enabled and auth is not yet ready", async () => { const state = { isAuthenticated: false, error: null, @@ -649,21 +607,17 @@ describe("AuthProvider", () => { checkAuthStatus: mockCheckAuthStatus, }); - const { result } = renderHook(() => useRootRouteContext(), { - wrapper: ({ children }) => ( - - {children} - - ), - }); + render( + +
Content
+
, + ); - expect(result.current).not.toBeNull(); - await result.current!.loader(new URL("http://localhost/")); - expect(mockCheckAuthStatus).toHaveBeenCalled(); - // autoLogin is no longer evaluated in the loader; it is handled - // reactively by the provider subscription so that it also covers - // mid-session expiry while idle on a page. + // login should not be called before the auth check completes expect(mockLogin).not.toHaveBeenCalled(); + await waitFor(() => { + expect(mockCheckAuthStatus).toHaveBeenCalled(); + }); }); it("should login when auth state changes to unauthenticated", async () => { diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx index aab8af36..9bafb66c 100644 --- a/packages/core/src/contexts/auth-context.tsx +++ b/packages/core/src/contexts/auth-context.tsx @@ -6,12 +6,13 @@ import { useEffect, useMemo, useRef, + use, + Suspense, } from "react"; import { createAuthClient as createAuthClientOriginal, type AuthClient, } from "@tailor-platform/auth-public-client"; -import { RootRouteContext } from "@/contexts/root-route-context"; // ============================================================================ // Auth Client @@ -43,6 +44,15 @@ export interface EnhancedAuthClient extends AuthClient { * Same signature as the standard `fetch` API. */ fetch: AuthClient["fetch"]; + + /** + * Promise that resolves when the OAuth callback has been handled. + * Non-null only when the client was created while an OAuth callback URL + * was present (i.e. `?code=` or `?error=` in window.location). + * Starts immediately at module load time, before any React render. + * @internal + */ + callbackPromise: Promise | null; } /** @@ -83,12 +93,28 @@ export function createAuthClient(config: AuthClientConfig): EnhancedAuthClient { const baseClient = createAuthClientOriginal(config); const { appUri } = config; + // Start OAuth callback handling immediately at module load time (before any + // React render). This is intentionally a side effect outside the React + // lifecycle — network I/O should not happen in the render phase. + const callbackPromise: Promise | null = (() => { + if (typeof window === "undefined") return null; + if (!isOAuthCallbackUrl(new URL(window.location.href))) return null; + return baseClient + .handleCallback() + .then(() => undefined) + .catch((error) => { + console.error("Failed to handle OAuth callback:", error); + }); + })(); + const enhancedClient: EnhancedAuthClient = { ...baseClient, getAppUri(): string { return appUri; }, + + callbackPromise, }; return enhancedClient; @@ -172,6 +198,22 @@ const isCurrentOAuthCallbackUrl = () => { return isOAuthCallbackUrl(new URL(window.location.href)); }; +/** + * Suspends children until the OAuth callback promise resolves. + * Used so that any component tree under AuthProvider is blocked from + * rendering until handleCallback() completes on the first load. + */ +const CallbackResolver = ({ + promise, + children, +}: { + promise: Promise; + children: React.ReactNode; +}) => { + use(promise); + return <>{children}; +}; + /** * Guard component that shows a fallback UI while auth is not ready or * not authenticated. Defined here so that the router layer does not @@ -211,6 +253,8 @@ type AuthProviderProps = { * Note: This prop only takes effect when AuthProvider wraps an AppShell * component. The guard is rendered by the router's root route element, * so it requires RouterContainer to be present in the component tree. + * For guarding components placed between AuthProvider and AppShell, + * rely on the authState from useAuth() directly. */ guardComponent?: () => React.ReactNode; }; @@ -359,57 +403,42 @@ export const AuthProvider = (props: React.PropsWithChildren) }); }, [ensureAuthInitialized]); - // The router loader is reserved for OAuth callback URLs. Its job is to - // finish the redirect handshake before rendering continues, while the - // ordinary "am I already signed in?" check stays with AuthProvider. - const authLoader = useCallback( - async (requestUrl: URL): Promise => { - // handleCallback() exchanges the callback parameters, stores the new - // session, and removes the temporary OAuth query parameters from the URL. - if (isOAuthCallbackUrl(requestUrl)) { - try { - await client.handleCallback(); - } catch (error) { - console.error("Failed to handle callback:", error); - } - return null; - } - - return null; - }, - [client], + // Handle OAuth callback URLs: block children from rendering until + // handleCallback() resolves. The promise is started in createAuthClient() + // at module load time — before any React render — so no render-phase + // side effects occur here. + const callbackPromise = props.client.callbackPromise; + + const { guardComponent } = props; + + const authContextValue = useMemo( + () => ({ + authState, + login: () => client.login(), + logout: () => client.logout(), + checkAuthStatus: () => client.checkAuthStatus(), + ready: () => client.ready(), + }), + [authState, client], ); - const guardComponent = props.guardComponent; - const guardComponentWrapper = useMemo( - () => - guardComponent - ? (children: React.ReactNode) => ( - {children} - ) - : undefined, - [guardComponent], - ); - - const rootRouteCtxValue = useMemo( - () => ({ loader: authLoader, wrapComponent: guardComponentWrapper }), - [authLoader, guardComponentWrapper], - ); + const resolvedChildren = + callbackPromise != null ? ( + + {props.children} + + ) : ( + props.children + ); return ( - - client.login(), - logout: () => client.logout(), - checkAuthStatus: () => client.checkAuthStatus(), - ready: () => client.ready(), - }} - > - {props.children} - - + + {guardComponent ? ( + {resolvedChildren} + ) : ( + resolvedChildren + )} + ); }; diff --git a/packages/core/src/contexts/root-route-context.tsx b/packages/core/src/contexts/root-route-context.tsx deleted file mode 100644 index d0aa8f6f..00000000 --- a/packages/core/src/contexts/root-route-context.tsx +++ /dev/null @@ -1,54 +0,0 @@ -import { createContext, useContext } from "react"; - -/** @internal */ -export type RootRouteLoaderFn = (requestUrl: URL) => Promise; - -/** @internal */ -export type RootRouteContextType = { - /** - * Runs in the react-router loader phase (before rendering). - * Used for side effects such as OAuth callback handling. - * - * ``` - * loader runs ──▶ rendering starts ──▶ wrapComponent applied - * (pre-render) (React lifecycle) (render phase) - * ``` - */ - loader: RootRouteLoaderFn; - /** - * Wraps the root route element during the React render phase. - * Used for UI concerns such as showing a guard component while - * auth is loading or the user is unauthenticated. - * - * ``` - * loader runs ──▶ rendering starts ──▶ wrapComponent applied - * (pre-render) (React lifecycle) (render phase) - * ``` - */ - wrapComponent?: (children: React.ReactNode) => React.ReactNode; -}; - -/** - * Internal context that allows providers (e.g. AuthProvider) to inject - * a loader function and an optional element wrapper into the router's - * root route. The router does not need to know which provider supplied - * the value — it only consumes this interface. - * - * Currently the only provider is AuthProvider. If additional providers - * need to participate in root loading in the future, a composition - * mechanism (e.g. accepting an array of loaders, or merging multiple - * contexts) would need to be introduced, since a single React context - * only holds one value. - * @internal - */ -export const RootRouteContext = createContext(null); - -/** - * Internal hook consumed by RouterContainer to obtain the root route - * context. Returns null when no provider (e.g. AuthProvider) supplies - * a value, making the integration optional. - * @internal - */ -export const useRootRouteContext = (): RootRouteContextType | null => { - return useContext(RootRouteContext); -}; diff --git a/packages/core/src/routing/router.test.tsx b/packages/core/src/routing/router.test.tsx index 7375b694..12e0315f 100644 --- a/packages/core/src/routing/router.test.tsx +++ b/packages/core/src/routing/router.test.tsx @@ -18,6 +18,7 @@ import { AuthProvider, type EnhancedAuthClient } from "@/contexts/auth-context"; afterEach(() => { cleanup(); + window.history.replaceState({}, "", "/"); }); const renderWithConfig = ({ @@ -428,6 +429,7 @@ const createMockAuthClient = ( getAuthHeaders: vi.fn(), getAppUri: vi.fn(() => "https://api.test.com"), getAuthHeadersForQuery: vi.fn(), + callbackPromise: null, ...overrides, } as EnhancedAuthClient; }; @@ -461,29 +463,26 @@ describe("RouterContainer with AuthProvider", () => { expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); }); - it("calls handleCallback when OAuth code is present in URL", async () => { + it("calls handleCallback when OAuth callbackPromise is set on client", async () => { const mockHandleCallback = vi.fn().mockResolvedValue(undefined); - const mockCheckAuthStatus = vi.fn().mockResolvedValue({ - isAuthenticated: true, - error: null, - isReady: true, - }); + // Simulate what createAuthClient does: kick off handleCallback() and store the promise. + const callbackPromise = Promise.resolve().then(() => mockHandleCallback()); const authClient = createMockAuthClient( { isAuthenticated: true, error: null, isReady: true }, - { - handleCallback: mockHandleCallback, - checkAuthStatus: mockCheckAuthStatus, - }, + { handleCallback: mockHandleCallback, callbackPromise }, ); - renderWithConfig({ - modules: [], - rootComponent: () =>
Home
, - initialEntries: ["/?code=auth-code-123&state=abc"], - authClient, + await act(async () => { + renderWithConfig({ + modules: [], + rootComponent: () =>
Home
, + initialEntries: ["/"], + authClient, + }); + await callbackPromise; }); - await screen.findByText("Home"); + expect(await screen.findByText("Home")).toBeDefined(); expect(mockHandleCallback).toHaveBeenCalled(); }); @@ -517,7 +516,9 @@ describe("RouterContainer with AuthProvider", () => { }); expect(await screen.findByText("Loading...")).toBeDefined(); - expect(createMemoryRouterSpy).toHaveBeenCalledTimes(1); + // With guardComponent now applied in AuthProvider (not the router), + // RouterContainer is not mounted until auth is ready. + expect(createMemoryRouterSpy).toHaveBeenCalledTimes(0); act(() => { snapshot = { @@ -531,6 +532,7 @@ describe("RouterContainer with AuthProvider", () => { }); expect(await screen.findByText("Home")).toBeDefined(); + // Router is created once when children first render after auth is ready. expect(createMemoryRouterSpy).toHaveBeenCalledTimes(1); } finally { createMemoryRouterSpy.mockRestore(); @@ -808,22 +810,27 @@ describe("RouterContainer with AuthProvider", () => { it("renders the correct page after OAuth callback with a specific path", async () => { const mockHandleCallback = vi.fn().mockResolvedValue(undefined); + // Simulate what createAuthClient does when ?code= is in the URL. + const callbackPromise = Promise.resolve().then(() => mockHandleCallback()); const authClient = createMockAuthClient( { isAuthenticated: true, error: null, isReady: true }, - { handleCallback: mockHandleCallback }, + { handleCallback: mockHandleCallback, callbackPromise }, ); - renderWithConfig({ - modules: [ - defineModule({ - path: "dashboard", - component: () =>
Dashboard
, - meta: { title: "Dashboard" }, - resources: [], - }), - ], - initialEntries: ["/dashboard?code=auth-code-123&state=abc"], - authClient, + await act(async () => { + renderWithConfig({ + modules: [ + defineModule({ + path: "dashboard", + component: () =>
Dashboard
, + meta: { title: "Dashboard" }, + resources: [], + }), + ], + initialEntries: ["/dashboard"], + authClient, + }); + await callbackPromise; }); expect(await screen.findByText("Dashboard")).toBeDefined(); diff --git a/packages/core/src/routing/router.tsx b/packages/core/src/routing/router.tsx index d5025684..e5ee76bb 100644 --- a/packages/core/src/routing/router.tsx +++ b/packages/core/src/routing/router.tsx @@ -1,50 +1,31 @@ import { type PropsWithChildren, type ReactNode, useMemo } from "react"; import { Outlet, createMemoryRouter, createBrowserRouter, RouterProvider } from "react-router"; -import type { LoaderFunctionArgs, RouteObject } from "react-router"; +import type { RouteObject } from "react-router"; import { createContentRoutes, RootComponentOption, wrapErrorBoundary } from "./routes"; import { useAppShellConfig, type RootConfiguration } from "@/contexts/appshell-context"; import { createNavItemsLoader } from "@/routing/navigation"; import type { Guard } from "@/resource"; -import { useRootRouteContext, type RootRouteContextType } from "@/contexts/root-route-context"; // ============================================================================ // Root Route // ============================================================================ /** - * Create the root route that combines root loader, navigation loading, - * error boundary, and optional element wrapping into a single RouteObject. - * - * When AuthProvider wraps AppShell, rootRouteCtx provides: - * - loader: runs before rendering (e.g. OAuth callback handling) - * - wrapComponent: wraps the root component (e.g. guard UI while loading) - * When AuthProvider is not used, rootRouteCtx is null and these are skipped. + * Create the root route that combines navigation loading and error boundary + * into a single RouteObject. */ const createRootRoute = (params: { configurations: RootConfiguration; - rootRouteCtx: RootRouteContextType | null; contentRoutes: Array; children: ReactNode; }): RouteObject => { - const { configurations, rootRouteCtx, contentRoutes, children } = params; + const { configurations, contentRoutes, children } = params; - // --- Loader: combine auth callback handling with navigation loading --- - const rootLoader = rootRouteCtx?.loader ?? null; - const { loaderID, loader: navLoader } = createNavItemsLoader({ + // --- Loader: load navigation items --- + const { loaderID, loader } = createNavItemsLoader({ modules: configurations.modules, locale: configurations.locale, }); - const loader = async (args: LoaderFunctionArgs) => { - if (rootLoader) { - const result = await rootLoader(new URL(args.request.url)); - if (result) return result; - } - return navLoader(); - }; - - // --- Element: apply wrapper when provided (e.g. auth guard) --- - const wrapComponent = rootRouteCtx?.wrapComponent; - const element = wrapComponent ? wrapComponent(children) : children; // --- Children: wrap with error boundary when configured --- const globalErrorBoundary = configurations.errorBoundary; @@ -61,7 +42,7 @@ const createRootRoute = (params: { return { id: loaderID, loader, - element, + element: children, children: routeChildren, // Hydration fallback is unused in CSR-only usage of AppShell. // Return null to silence hydration warnings. @@ -90,7 +71,6 @@ export type RouterContainerProps = export const RouterContainer = (props: PropsWithChildren) => { const { rootComponent, children } = props; const { configurations } = useAppShellConfig(); - const rootRouteCtx = useRootRouteContext(); const contentRoutes = useMemo( () => createContentRoutes({ @@ -106,12 +86,11 @@ export const RouterContainer = (props: PropsWithChildren) [ createRootRoute({ configurations, - rootRouteCtx, contentRoutes, children, }), ] satisfies Array, - [configurations, rootRouteCtx, contentRoutes, children], + [configurations, contentRoutes, children], ); const basename = configurations.basePath ? "/" + configurations.basePath : undefined; From cc3fcaaa54c2bd294277791369c77b30f8a058f7 Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Sun, 19 Apr 2026 17:53:51 +0900 Subject: [PATCH 10/15] Format --- packages/core/src/contexts/auth-context.tsx | 23 +++++++++------------ 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx index 9bafb66c..eec29c74 100644 --- a/packages/core/src/contexts/auth-context.tsx +++ b/packages/core/src/contexts/auth-context.tsx @@ -408,8 +408,14 @@ export const AuthProvider = (props: React.PropsWithChildren) // at module load time — before any React render — so no render-phase // side effects occur here. const callbackPromise = props.client.callbackPromise; - - const { guardComponent } = props; + const resolvedChildren = + callbackPromise != null ? ( + + {props.children} + + ) : ( + props.children + ); const authContextValue = useMemo( () => ({ @@ -422,19 +428,10 @@ export const AuthProvider = (props: React.PropsWithChildren) [authState, client], ); - const resolvedChildren = - callbackPromise != null ? ( - - {props.children} - - ) : ( - props.children - ); - return ( - {guardComponent ? ( - {resolvedChildren} + {props.guardComponent ? ( + {resolvedChildren} ) : ( resolvedChildren )} From 4bd795a07378a61357b2308337f2cc0adabb34c8 Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Sun, 19 Apr 2026 21:33:18 +0900 Subject: [PATCH 11/15] WIP --- .../core/src/contexts/auth-context.test.tsx | 60 +++++++++++++++++++ packages/core/src/contexts/auth-context.tsx | 12 +++- 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx index f9611872..c45d611b 100644 --- a/packages/core/src/contexts/auth-context.test.tsx +++ b/packages/core/src/contexts/auth-context.test.tsx @@ -316,6 +316,66 @@ describe("AuthProvider", () => { expect(mockHandleCallback).toHaveBeenCalled(); }); + it("should not blank-screen when guardComponent is provided and callbackPromise settles after auth_state_changed", async () => { + // Regression test: when guardComponent is provided, AuthGuard gates rendering + // via isReady/isAuthenticated. Using CallbackResolver on top causes a race: + // auth_state_changed fires before the callbackPromise chain fully settles, + // so CallbackResolver suspends (fallback=null) inside a sync-lane render. + // Fix: skip CallbackResolver entirely when guardComponent is present. + let resolveCallback!: () => void; + const callbackPromise = new Promise((resolve) => { + resolveCallback = resolve; + }); + + const listeners: Array<(event: { type: string }) => void> = []; + let snapshot = { + isAuthenticated: false, + error: null as string | null, + isReady: false, + }; + const mockClient = createMockAuthClient(snapshot, { + getState: vi.fn(() => snapshot), + callbackPromise, + addEventListener: vi.fn((listener) => { + listeners.push(listener); + return () => { + const idx = listeners.indexOf(listener); + if (idx >= 0) listeners.splice(idx, 1); + }; + }), + }); + + render( +
Loading...
}> +
Content
+
, + ); + + // Guard is shown while not ready + expect(screen.getByText("Loading...")).toBeDefined(); + expect(screen.queryByText("Content")).toBeNull(); + + // Simulate handleCallbackInternal completing: state updates then + // auth_state_changed fires — callbackPromise is still pending at this point. + act(() => { + snapshot = { isAuthenticated: true, error: null, isReady: true }; + for (const listener of listeners) { + listener({ type: "auth_state_changed" }); + } + }); + + // Content must be visible immediately — no blank screen despite + // callbackPromise still being unresolved. + expect(screen.getByText("Content")).toBeDefined(); + expect(screen.queryByText("Loading...")).toBeNull(); + + // Resolving the promise afterwards should not break anything + act(() => { + resolveCallback(); + }); + expect(screen.getByText("Content")).toBeDefined(); + }); + it("should be authenticated when logged in", async () => { const state = { isAuthenticated: true, diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx index eec29c74..dc3b13e1 100644 --- a/packages/core/src/contexts/auth-context.tsx +++ b/packages/core/src/contexts/auth-context.tsx @@ -407,9 +407,19 @@ export const AuthProvider = (props: React.PropsWithChildren) // handleCallback() resolves. The promise is started in createAuthClient() // at module load time — before any React render — so no render-phase // side effects occur here. + // + // When guardComponent is provided, AuthGuard already gates children until + // isReady && isAuthenticated, which is only set inside handleCallbackInternal + // *before* auth_state_changed fires. Wrapping with CallbackResolver in this + // case causes a race condition: auth_state_changed fires (triggering AuthGuard + // to show children) a few microtasks before the callbackPromise chain fully + // settles. CallbackResolver then suspends inside a sync-lane render, showing + // fallback={null} — a blank screen — and Suspense retry is unreliable in that + // context. Since AuthGuard already provides the required guard, skip + // CallbackResolver entirely when guardComponent is present. const callbackPromise = props.client.callbackPromise; const resolvedChildren = - callbackPromise != null ? ( + callbackPromise != null && !props.guardComponent ? ( {props.children} From 0b9a39d0feb1b292ce249aa83b36074301aac061 Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Sun, 19 Apr 2026 21:41:43 +0900 Subject: [PATCH 12/15] Revert "WIP" This reverts commit 4bd795a07378a61357b2308337f2cc0adabb34c8. --- .../core/src/contexts/auth-context.test.tsx | 60 ------------------- packages/core/src/contexts/auth-context.tsx | 12 +--- 2 files changed, 1 insertion(+), 71 deletions(-) diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx index c45d611b..f9611872 100644 --- a/packages/core/src/contexts/auth-context.test.tsx +++ b/packages/core/src/contexts/auth-context.test.tsx @@ -316,66 +316,6 @@ describe("AuthProvider", () => { expect(mockHandleCallback).toHaveBeenCalled(); }); - it("should not blank-screen when guardComponent is provided and callbackPromise settles after auth_state_changed", async () => { - // Regression test: when guardComponent is provided, AuthGuard gates rendering - // via isReady/isAuthenticated. Using CallbackResolver on top causes a race: - // auth_state_changed fires before the callbackPromise chain fully settles, - // so CallbackResolver suspends (fallback=null) inside a sync-lane render. - // Fix: skip CallbackResolver entirely when guardComponent is present. - let resolveCallback!: () => void; - const callbackPromise = new Promise((resolve) => { - resolveCallback = resolve; - }); - - const listeners: Array<(event: { type: string }) => void> = []; - let snapshot = { - isAuthenticated: false, - error: null as string | null, - isReady: false, - }; - const mockClient = createMockAuthClient(snapshot, { - getState: vi.fn(() => snapshot), - callbackPromise, - addEventListener: vi.fn((listener) => { - listeners.push(listener); - return () => { - const idx = listeners.indexOf(listener); - if (idx >= 0) listeners.splice(idx, 1); - }; - }), - }); - - render( -
Loading...
}> -
Content
-
, - ); - - // Guard is shown while not ready - expect(screen.getByText("Loading...")).toBeDefined(); - expect(screen.queryByText("Content")).toBeNull(); - - // Simulate handleCallbackInternal completing: state updates then - // auth_state_changed fires — callbackPromise is still pending at this point. - act(() => { - snapshot = { isAuthenticated: true, error: null, isReady: true }; - for (const listener of listeners) { - listener({ type: "auth_state_changed" }); - } - }); - - // Content must be visible immediately — no blank screen despite - // callbackPromise still being unresolved. - expect(screen.getByText("Content")).toBeDefined(); - expect(screen.queryByText("Loading...")).toBeNull(); - - // Resolving the promise afterwards should not break anything - act(() => { - resolveCallback(); - }); - expect(screen.getByText("Content")).toBeDefined(); - }); - it("should be authenticated when logged in", async () => { const state = { isAuthenticated: true, diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx index dc3b13e1..eec29c74 100644 --- a/packages/core/src/contexts/auth-context.tsx +++ b/packages/core/src/contexts/auth-context.tsx @@ -407,19 +407,9 @@ export const AuthProvider = (props: React.PropsWithChildren) // handleCallback() resolves. The promise is started in createAuthClient() // at module load time — before any React render — so no render-phase // side effects occur here. - // - // When guardComponent is provided, AuthGuard already gates children until - // isReady && isAuthenticated, which is only set inside handleCallbackInternal - // *before* auth_state_changed fires. Wrapping with CallbackResolver in this - // case causes a race condition: auth_state_changed fires (triggering AuthGuard - // to show children) a few microtasks before the callbackPromise chain fully - // settles. CallbackResolver then suspends inside a sync-lane render, showing - // fallback={null} — a blank screen — and Suspense retry is unreliable in that - // context. Since AuthGuard already provides the required guard, skip - // CallbackResolver entirely when guardComponent is present. const callbackPromise = props.client.callbackPromise; const resolvedChildren = - callbackPromise != null && !props.guardComponent ? ( + callbackPromise != null ? ( {props.children} From ea4e16c6f4d6f476db5d3c3c9ff066fc6121cd97 Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Sun, 19 Apr 2026 23:03:54 +0900 Subject: [PATCH 13/15] Refine auth callback status handling --- .../core/src/contexts/auth-context.test.tsx | 130 +++++++++--- packages/core/src/contexts/auth-context.tsx | 185 ++++++++++++------ packages/core/src/routing/router.test.tsx | 132 +++++++++---- 3 files changed, 321 insertions(+), 126 deletions(-) diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx index f9611872..cc84a62e 100644 --- a/packages/core/src/contexts/auth-context.test.tsx +++ b/packages/core/src/contexts/auth-context.test.tsx @@ -51,14 +51,17 @@ describe("AuthProvider", () => { return handleCallbackInFlight; } - const callbackPromise = Promise.resolve(baseHandleCallback()).finally(() => { - handleCallbackInFlight = null; - }); + const callbackPromise = Promise.resolve(baseHandleCallback()).finally( + () => { + handleCallbackInFlight = null; + }, + ); handleCallbackInFlight = callbackPromise; return callbackPromise; }); - const { handleCallback: _ignoredHandleCallback, ...otherOverrides } = overrides ?? {}; + const { handleCallback: _ignoredHandleCallback, ...otherOverrides } = + overrides ?? {}; return { getState: vi.fn(() => state), @@ -77,7 +80,8 @@ describe("AuthProvider", () => { getAuthHeaders: vi.fn(), fetch: vi.fn(), getAppUri: vi.fn(() => "https://api.test.com"), - callbackPromise: null, + getCallbackStatusSnapshot: vi.fn(() => "idle"), + subscribeCallbackStatus: vi.fn(() => () => {}), ...otherOverrides, handleCallback, } as EnhancedAuthClient; @@ -288,34 +292,69 @@ describe("AuthProvider", () => { }); }); - it("should suspend children until callbackPromise resolves when set on client", async () => { + it("should wait to render children until callback settles when set on client", async () => { const mockHandleCallback = vi.fn().mockResolvedValue(undefined); - // Simulate what createAuthClient does when ?code= is in the URL: - // kick off handleCallback() and store the promise on the client. - const callbackPromise = Promise.resolve().then(() => mockHandleCallback()); + let callbackStatus: "idle" | "pending" | "resolved" | "rejected" = + "pending"; + let callbackStatusListener: (() => void) | undefined; const state = { isAuthenticated: true, error: null, isReady: true, }; const mockClient = createMockAuthClient(state, { - handleCallback: mockHandleCallback, - callbackPromise, + getCallbackStatusSnapshot: vi.fn(() => callbackStatus), + subscribeCallbackStatus: vi.fn((listener: () => void) => { + callbackStatusListener = listener; + return () => { + callbackStatusListener = undefined; + }; + }), }); + render( + +
Content
+
, + ); + + expect(screen.queryByText("Content")).toBeNull(); + await act(async () => { - render( - -
Content
-
, - ); - await callbackPromise; + await mockHandleCallback(); + callbackStatus = "resolved"; + callbackStatusListener?.(); }); expect(screen.getByText("Content")).toBeDefined(); expect(mockHandleCallback).toHaveBeenCalled(); }); + it("should render guarded children while callback is pending", async () => { + const pendingCallbackStatus: ReturnType< + EnhancedAuthClient["getCallbackStatusSnapshot"] + > = "pending"; + const state = { + isAuthenticated: true, + error: null, + isReady: true, + }; + const mockClient = createMockAuthClient(state, { + getCallbackStatusSnapshot: vi.fn(() => pendingCallbackStatus), + }); + + render( + +
Protected Content
+
, + ); + + await waitFor(() => { + expect(screen.getByText("Protected Content")).toBeDefined(); + }); + expect(screen.queryByText("Please log in")).toBeNull(); + }); + it("should be authenticated when logged in", async () => { const state = { isAuthenticated: true, @@ -331,7 +370,9 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); await waitFor(() => { @@ -357,7 +398,9 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); await waitFor(() => { @@ -385,7 +428,9 @@ describe("AuthProvider", () => { const mockClient = createMockAuthClient(state); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); await waitFor(() => { @@ -412,7 +457,9 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); await waitFor(() => { @@ -440,7 +487,9 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); await waitFor(() => { @@ -500,7 +549,9 @@ describe("AuthProvider", () => { }); it("should resolve suspense when auth becomes ready", async () => { - let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; + let authEventListener: + | ((event: { type: string; data?: unknown }) => void) + | undefined; const mockAddEventListener = vi.fn( (listener: (event: { type: string; data?: unknown }) => void) => { @@ -527,7 +578,12 @@ describe("AuthProvider", () => { const TestComponent = () => { const { isAuthenticated } = useAuthSuspense(); - return
Content Loaded: {isAuthenticated ? "authenticated" : "not authenticated"}
; + return ( +
+ Content Loaded:{" "} + {isAuthenticated ? "authenticated" : "not authenticated"} +
+ ); }; render( @@ -571,7 +627,11 @@ describe("AuthProvider", () => { const TestComponent = () => { const { isAuthenticated } = useAuthSuspense(); - return
Loaded: {isAuthenticated ? "authenticated" : "unauthenticated"}
; + return ( +
+ Loaded: {isAuthenticated ? "authenticated" : "unauthenticated"} +
+ ); }; render( @@ -621,7 +681,9 @@ describe("AuthProvider", () => { }); it("should login when auth state changes to unauthenticated", async () => { - let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; + let authEventListener: + | ((event: { type: string; data?: unknown }) => void) + | undefined; const mockAddEventListener = vi.fn( (listener: (event: { type: string; data?: unknown }) => void) => { @@ -726,7 +788,9 @@ describe("AuthProvider", () => { it("should not login while the current URL is an OAuth callback", async () => { window.history.replaceState({}, "", "/?code=auth-code-123&state=abc"); - let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; + let authEventListener: + | ((event: { type: string; data?: unknown }) => void) + | undefined; const mockAddEventListener = vi.fn( (listener: (event: { type: string; data?: unknown }) => void) => { @@ -779,7 +843,9 @@ describe("AuthProvider", () => { describe("event listeners", () => { it("should listen to auth state changes via useSyncExternalStore", async () => { - let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; + let authEventListener: + | ((event: { type: string; data?: unknown }) => void) + | undefined; const mockAddEventListener = vi.fn( (listener: (event: { type: string; data?: unknown }) => void) => { @@ -803,7 +869,9 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); await waitFor(() => { @@ -846,7 +914,9 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => {children}, + wrapper: ({ children }) => ( + {children} + ), }); await waitFor(() => { diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx index eec29c74..7aa0340f 100644 --- a/packages/core/src/contexts/auth-context.tsx +++ b/packages/core/src/contexts/auth-context.tsx @@ -6,8 +6,6 @@ import { useEffect, useMemo, useRef, - use, - Suspense, } from "react"; import { createAuthClient as createAuthClientOriginal, @@ -30,6 +28,11 @@ export interface AuthClientConfig { redirectUri?: string; } +/** + * Internal type for tracking OAuth callback handling status. + */ +type CallbackStatus = "idle" | "pending" | "resolved" | "rejected"; + /** * Enhanced auth client with additional helper methods */ @@ -46,15 +49,64 @@ export interface EnhancedAuthClient extends AuthClient { fetch: AuthClient["fetch"]; /** - * Promise that resolves when the OAuth callback has been handled. - * Non-null only when the client was created while an OAuth callback URL - * was present (i.e. `?code=` or `?error=` in window.location). - * Starts immediately at module load time, before any React render. + * Returns the current OAuth callback handling state. + * @internal + */ + getCallbackStatusSnapshot(): CallbackStatus; + + /** + * Subscribe to callback settlement changes. * @internal */ - callbackPromise: Promise | null; + subscribeCallbackStatus(listener: () => void): () => void; } +/** + * Small external-store manager dedicated to the OAuth callback lifecycle. + * + * AuthProvider reads this state via useSyncExternalStore so it can delay + * rendering while the login callback is still being processed, without + * coupling that control flow to the auth client's own auth_state_changed + * events. The manager exposes callback-specific transitions instead of a + * generic setter so createAuthClient can describe the callback flow directly. + */ +const createCallbackStatusManager = () => { + let callbackStatus: CallbackStatus = "idle"; + const CALLBACK_STATUS_CHANGE_EVENT = "callbackstatuschange"; + const target = new EventTarget(); + + const updateStatus = (nextStatus: CallbackStatus) => { + callbackStatus = nextStatus; + target.dispatchEvent(new Event(CALLBACK_STATUS_CHANGE_EVENT)); + }; + + return { + getSnapshot: () => callbackStatus, + start: () => { + updateStatus("pending"); + + return { + resolve: () => { + updateStatus("resolved"); + }, + reject: () => { + updateStatus("rejected"); + }, + }; + }, + subscribe: (listener: () => void) => { + const eventListener: EventListener = () => { + listener(); + }; + + target.addEventListener(CALLBACK_STATUS_CHANGE_EVENT, eventListener); + return () => { + target.removeEventListener(CALLBACK_STATUS_CHANGE_EVENT, eventListener); + }; + }, + }; +}; + /** * Create an enhanced authentication client. * @@ -92,20 +144,26 @@ export interface EnhancedAuthClient extends AuthClient { export function createAuthClient(config: AuthClientConfig): EnhancedAuthClient { const baseClient = createAuthClientOriginal(config); const { appUri } = config; + const callbackManager = createCallbackStatusManager(); // Start OAuth callback handling immediately at module load time (before any // React render). This is intentionally a side effect outside the React // lifecycle — network I/O should not happen in the render phase. - const callbackPromise: Promise | null = (() => { - if (typeof window === "undefined") return null; - if (!isOAuthCallbackUrl(new URL(window.location.href))) return null; - return baseClient - .handleCallback() - .then(() => undefined) - .catch((error) => { - console.error("Failed to handle OAuth callback:", error); - }); - })(); + if (typeof window !== "undefined") { + const currentUrl = new URL(window.location.href); + + if (isOAuthCallbackUrl(currentUrl)) { + const { resolve, reject } = callbackManager.start(); + + baseClient + .handleCallback() + .then(resolve) + .catch((error) => { + reject(); + console.error("Failed to handle OAuth callback:", error); + }); + } + } const enhancedClient: EnhancedAuthClient = { ...baseClient, @@ -114,7 +172,13 @@ export function createAuthClient(config: AuthClientConfig): EnhancedAuthClient { return appUri; }, - callbackPromise, + getCallbackStatusSnapshot() { + return callbackManager.getSnapshot(); + }, + + subscribeCallbackStatus(listener) { + return callbackManager.subscribe(listener); + }, }; return enhancedClient; @@ -198,22 +262,6 @@ const isCurrentOAuthCallbackUrl = () => { return isOAuthCallbackUrl(new URL(window.location.href)); }; -/** - * Suspends children until the OAuth callback promise resolves. - * Used so that any component tree under AuthProvider is blocked from - * rendering until handleCallback() completes on the first load. - */ -const CallbackResolver = ({ - promise, - children, -}: { - promise: Promise; - children: React.ReactNode; -}) => { - use(promise); - return <>{children}; -}; - /** * Guard component that shows a fallback UI while auth is not ready or * not authenticated. Defined here so that the router layer does not @@ -227,6 +275,7 @@ const AuthGuard = ({ children: React.ReactNode; }) => { const { isReady, isAuthenticated } = useAuth(); + if (!isReady || !isAuthenticated) { return guardComponent(); } @@ -267,7 +316,10 @@ type AuthProviderProps = { * - initial deferred auto-login attempt * - duplicate login prevention */ -const useAutoLogin = (props: { client: EnhancedAuthClient; enabled?: boolean }) => { +const useAutoLogin = (props: { + client: EnhancedAuthClient; + enabled?: boolean; +}) => { // Prevent duplicate login redirects when multiple auth_state_changed // events fire before the first login attempt settles. const loginInFlightRef = useRef | null>(null); @@ -287,6 +339,7 @@ const useAutoLogin = (props: { client: EnhancedAuthClient; enabled?: boolean }) loginInFlightRef.current = props.client .login() + .then(() => undefined) .catch((error) => { console.error("Failed to auto-login after session expiry:", error); }) @@ -330,7 +383,10 @@ export const useEnsureAuthInitialized = (client: EnhancedAuthClient) => { const initInFlightRef = useRef | null>(null); const ensureInitialized = useCallback(async (): Promise => { - if (isCurrentOAuthCallbackUrl() || client.getState().isReady) { + const authState = client.getState(); + const isCallbackUrl = isCurrentOAuthCallbackUrl(); + + if (isCallbackUrl || authState.isReady) { return; } @@ -341,6 +397,9 @@ export const useEnsureAuthInitialized = (client: EnhancedAuthClient) => { initInFlightRef.current = client .checkAuthStatus() .then(() => undefined) + .catch((error) => { + throw error; + }) .finally(() => { initInFlightRef.current = null; }); @@ -351,6 +410,24 @@ export const useEnsureAuthInitialized = (client: EnhancedAuthClient) => { return ensureInitialized; }; +/** + * Reads the OAuth callback handling state from the auth client via + * useSyncExternalStore so AuthProvider can coordinate rendering while the + * callback exchange is still in flight. + */ +const useCallbackStatus = (client: EnhancedAuthClient) => { + const subscribe = useCallback( + (notify: () => void) => client.subscribeCallbackStatus(notify), + [client], + ); + const getSnapshot = useCallback( + () => client.getCallbackStatusSnapshot(), + [client], + ); + + return useSyncExternalStore(subscribe, getSnapshot); +}; + /** * Authentication provider component. * @@ -375,7 +452,9 @@ export const useEnsureAuthInitialized = (client: EnhancedAuthClient) => { * } * ``` */ -export const AuthProvider = (props: React.PropsWithChildren) => { +export const AuthProvider = ( + props: React.PropsWithChildren, +) => { const client = props.client; // Set up auth state subscription for auto-login orchestration @@ -384,10 +463,8 @@ export const AuthProvider = (props: React.PropsWithChildren) enabled: props.autoLogin, }); - // Get current state snapshot (no cache needed - client returns same reference) + // Use useSyncExternalStore for state management from auth client. const getSnapshot = useCallback(() => client.getState(), [client]); - - // Use useSyncExternalStore for state management const authState = useSyncExternalStore(subscribeAuthState, getSnapshot); // Prepare a shared initialization function so AuthProvider can start the @@ -401,21 +478,15 @@ export const AuthProvider = (props: React.PropsWithChildren) ensureAuthInitialized().catch((error) => { console.error("Failed to check auth status:", error); }); - }, [ensureAuthInitialized]); + }, [client, ensureAuthInitialized]); - // Handle OAuth callback URLs: block children from rendering until - // handleCallback() resolves. The promise is started in createAuthClient() - // at module load time — before any React render — so no render-phase - // side effects occur here. - const callbackPromise = props.client.callbackPromise; + // While handling an OAuth callback, keep unguarded children hidden until + // the callback settles. Guarded trees already wait on auth state instead. + const callbackStatus = useCallbackStatus(client); const resolvedChildren = - callbackPromise != null ? ( - - {props.children} - - ) : ( - props.children - ); + callbackStatus === "pending" && props.guardComponent == null + ? null + : props.children; const authContextValue = useMemo( () => ({ @@ -431,7 +502,9 @@ export const AuthProvider = (props: React.PropsWithChildren) return ( {props.guardComponent ? ( - {resolvedChildren} + + {resolvedChildren} + ) : ( resolvedChildren )} @@ -445,7 +518,9 @@ export const AuthProvider = (props: React.PropsWithChildren) const useAuthContext = () => { const context = useContext(AuthContext); if (!context) { - throw new Error("useAuth/useAuthSuspense must be used within an AuthProvider"); + throw new Error( + "useAuth/useAuthSuspense must be used within an AuthProvider", + ); } return context; }; diff --git a/packages/core/src/routing/router.test.tsx b/packages/core/src/routing/router.test.tsx index 12e0315f..59dd14e2 100644 --- a/packages/core/src/routing/router.test.tsx +++ b/packages/core/src/routing/router.test.tsx @@ -1,7 +1,17 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { + act, + cleanup, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/react"; import { RouterContainer } from "./router"; -import { AppShellConfigContext, AppShellDataContext } from "@/contexts/appshell-context"; +import { + AppShellConfigContext, + AppShellDataContext, +} from "@/contexts/appshell-context"; import * as ReactRouter from "react-router"; import { Link, Outlet, useNavigate } from "react-router"; import { @@ -55,7 +65,11 @@ const renderWithConfig = ({ const tree = ( - + @@ -64,7 +78,11 @@ const renderWithConfig = ({ return render( authClient ? ( - + {tree} ) : ( @@ -214,7 +232,9 @@ describe("RouterContainer (memory)", () => { initialEntries: ["/dashboard"], }); - expect(await screen.findByRole("alert", { name: "default-error-boundary" })).toBeDefined(); + expect( + await screen.findByRole("alert", { name: "default-error-boundary" }), + ).toBeDefined(); cleanup(); @@ -223,7 +243,9 @@ describe("RouterContainer (memory)", () => { initialEntries: ["/dashboard/overview"], }); - expect(await screen.findByRole("alert", { name: "default-error-boundary" })).toBeDefined(); + expect( + await screen.findByRole("alert", { name: "default-error-boundary" }), + ).toBeDefined(); } finally { consoleSpy.mockRestore(); } @@ -235,7 +257,9 @@ describe("RouterContainer (memory)", () => { initialEntries: ["/unknown-path"], }); - expect(await screen.findByRole("alert", { name: "default-error-boundary" })).toBeDefined(); + expect( + await screen.findByRole("alert", { name: "default-error-boundary" }), + ).toBeDefined(); expect(await screen.findByText("404 Not Found")).toBeDefined(); }); @@ -366,7 +390,9 @@ describe("RouterContainer (memory)", () => { initialEntries: ["/admin"], }); - expect(await screen.findByRole("alert", { name: "default-error-boundary" })).toBeDefined(); + expect( + await screen.findByRole("alert", { name: "default-error-boundary" }), + ).toBeDefined(); expect(await screen.findByText("404 Not Found")).toBeDefined(); }); @@ -429,7 +455,8 @@ const createMockAuthClient = ( getAuthHeaders: vi.fn(), getAppUri: vi.fn(() => "https://api.test.com"), getAuthHeadersForQuery: vi.fn(), - callbackPromise: null, + getCallbackStatusSnapshot: vi.fn(() => "idle"), + subscribeCallbackStatus: vi.fn(() => () => {}), ...overrides, } as EnhancedAuthClient; }; @@ -463,27 +490,38 @@ describe("RouterContainer with AuthProvider", () => { expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); }); - it("calls handleCallback when OAuth callbackPromise is set on client", async () => { - const mockHandleCallback = vi.fn().mockResolvedValue(undefined); - // Simulate what createAuthClient does: kick off handleCallback() and store the promise. - const callbackPromise = Promise.resolve().then(() => mockHandleCallback()); + it("waits to render until callback handling finishes", async () => { + let callbackStatus: "idle" | "pending" | "resolved" | "rejected" = + "pending"; + let callbackStatusListener: (() => void) | undefined; const authClient = createMockAuthClient( { isAuthenticated: true, error: null, isReady: true }, - { handleCallback: mockHandleCallback, callbackPromise }, + { + getCallbackStatusSnapshot: vi.fn(() => callbackStatus), + subscribeCallbackStatus: vi.fn((listener: () => void) => { + callbackStatusListener = listener; + return () => { + callbackStatusListener = undefined; + }; + }), + }, ); + renderWithConfig({ + modules: [], + rootComponent: () =>
Home
, + initialEntries: ["/"], + authClient, + }); + + expect(screen.queryByText("Home")).toBeNull(); + await act(async () => { - renderWithConfig({ - modules: [], - rootComponent: () =>
Home
, - initialEntries: ["/"], - authClient, - }); - await callbackPromise; + callbackStatus = "resolved"; + callbackStatusListener?.(); }); expect(await screen.findByText("Home")).toBeDefined(); - expect(mockHandleCallback).toHaveBeenCalled(); }); it("does not recreate the router when auth state changes", async () => { @@ -809,32 +847,41 @@ describe("RouterContainer with AuthProvider", () => { }); it("renders the correct page after OAuth callback with a specific path", async () => { - const mockHandleCallback = vi.fn().mockResolvedValue(undefined); - // Simulate what createAuthClient does when ?code= is in the URL. - const callbackPromise = Promise.resolve().then(() => mockHandleCallback()); + let callbackStatus: "idle" | "pending" | "resolved" | "rejected" = + "pending"; + let callbackStatusListener: (() => void) | undefined; const authClient = createMockAuthClient( { isAuthenticated: true, error: null, isReady: true }, - { handleCallback: mockHandleCallback, callbackPromise }, + { + getCallbackStatusSnapshot: vi.fn(() => callbackStatus), + subscribeCallbackStatus: vi.fn((listener: () => void) => { + callbackStatusListener = listener; + return () => { + callbackStatusListener = undefined; + }; + }), + }, ); + renderWithConfig({ + modules: [ + defineModule({ + path: "dashboard", + component: () =>
Dashboard
, + meta: { title: "Dashboard" }, + resources: [], + }), + ], + initialEntries: ["/dashboard"], + authClient, + }); + await act(async () => { - renderWithConfig({ - modules: [ - defineModule({ - path: "dashboard", - component: () =>
Dashboard
, - meta: { title: "Dashboard" }, - resources: [], - }), - ], - initialEntries: ["/dashboard"], - authClient, - }); - await callbackPromise; + callbackStatus = "resolved"; + callbackStatusListener?.(); }); expect(await screen.findByText("Dashboard")).toBeDefined(); - expect(mockHandleCallback).toHaveBeenCalled(); }); it("applies both auth guard and route-level guards correctly", async () => { @@ -909,7 +956,10 @@ describe("RouterContainer with AuthProvider", () => { return (

Dashboard

- From 369c6851ac7f2523df491c0989e479c821767432 Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Sun, 19 Apr 2026 23:06:04 +0900 Subject: [PATCH 14/15] Format files --- .../core/src/contexts/auth-context.test.tsx | 76 +++++-------------- packages/core/src/contexts/auth-context.tsx | 26 ++----- packages/core/src/routing/router.test.tsx | 53 +++---------- 3 files changed, 37 insertions(+), 118 deletions(-) diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx index cc84a62e..6ffff7fd 100644 --- a/packages/core/src/contexts/auth-context.test.tsx +++ b/packages/core/src/contexts/auth-context.test.tsx @@ -51,17 +51,14 @@ describe("AuthProvider", () => { return handleCallbackInFlight; } - const callbackPromise = Promise.resolve(baseHandleCallback()).finally( - () => { - handleCallbackInFlight = null; - }, - ); + const callbackPromise = Promise.resolve(baseHandleCallback()).finally(() => { + handleCallbackInFlight = null; + }); handleCallbackInFlight = callbackPromise; return callbackPromise; }); - const { handleCallback: _ignoredHandleCallback, ...otherOverrides } = - overrides ?? {}; + const { handleCallback: _ignoredHandleCallback, ...otherOverrides } = overrides ?? {}; return { getState: vi.fn(() => state), @@ -294,8 +291,7 @@ describe("AuthProvider", () => { it("should wait to render children until callback settles when set on client", async () => { const mockHandleCallback = vi.fn().mockResolvedValue(undefined); - let callbackStatus: "idle" | "pending" | "resolved" | "rejected" = - "pending"; + let callbackStatus: "idle" | "pending" | "resolved" | "rejected" = "pending"; let callbackStatusListener: (() => void) | undefined; const state = { isAuthenticated: true, @@ -331,9 +327,8 @@ describe("AuthProvider", () => { }); it("should render guarded children while callback is pending", async () => { - const pendingCallbackStatus: ReturnType< - EnhancedAuthClient["getCallbackStatusSnapshot"] - > = "pending"; + const pendingCallbackStatus: ReturnType = + "pending"; const state = { isAuthenticated: true, error: null, @@ -370,9 +365,7 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); await waitFor(() => { @@ -398,9 +391,7 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); await waitFor(() => { @@ -428,9 +419,7 @@ describe("AuthProvider", () => { const mockClient = createMockAuthClient(state); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); await waitFor(() => { @@ -457,9 +446,7 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); await waitFor(() => { @@ -487,9 +474,7 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); await waitFor(() => { @@ -549,9 +534,7 @@ describe("AuthProvider", () => { }); it("should resolve suspense when auth becomes ready", async () => { - let authEventListener: - | ((event: { type: string; data?: unknown }) => void) - | undefined; + let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; const mockAddEventListener = vi.fn( (listener: (event: { type: string; data?: unknown }) => void) => { @@ -578,12 +561,7 @@ describe("AuthProvider", () => { const TestComponent = () => { const { isAuthenticated } = useAuthSuspense(); - return ( -
- Content Loaded:{" "} - {isAuthenticated ? "authenticated" : "not authenticated"} -
- ); + return
Content Loaded: {isAuthenticated ? "authenticated" : "not authenticated"}
; }; render( @@ -627,11 +605,7 @@ describe("AuthProvider", () => { const TestComponent = () => { const { isAuthenticated } = useAuthSuspense(); - return ( -
- Loaded: {isAuthenticated ? "authenticated" : "unauthenticated"} -
- ); + return
Loaded: {isAuthenticated ? "authenticated" : "unauthenticated"}
; }; render( @@ -681,9 +655,7 @@ describe("AuthProvider", () => { }); it("should login when auth state changes to unauthenticated", async () => { - let authEventListener: - | ((event: { type: string; data?: unknown }) => void) - | undefined; + let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; const mockAddEventListener = vi.fn( (listener: (event: { type: string; data?: unknown }) => void) => { @@ -788,9 +760,7 @@ describe("AuthProvider", () => { it("should not login while the current URL is an OAuth callback", async () => { window.history.replaceState({}, "", "/?code=auth-code-123&state=abc"); - let authEventListener: - | ((event: { type: string; data?: unknown }) => void) - | undefined; + let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; const mockAddEventListener = vi.fn( (listener: (event: { type: string; data?: unknown }) => void) => { @@ -843,9 +813,7 @@ describe("AuthProvider", () => { describe("event listeners", () => { it("should listen to auth state changes via useSyncExternalStore", async () => { - let authEventListener: - | ((event: { type: string; data?: unknown }) => void) - | undefined; + let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; const mockAddEventListener = vi.fn( (listener: (event: { type: string; data?: unknown }) => void) => { @@ -869,9 +837,7 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); await waitFor(() => { @@ -914,9 +880,7 @@ describe("AuthProvider", () => { }); const { result } = renderHook(() => useAuth(), { - wrapper: ({ children }) => ( - {children} - ), + wrapper: ({ children }) => {children}, }); await waitFor(() => { diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx index 7aa0340f..61db4a4e 100644 --- a/packages/core/src/contexts/auth-context.tsx +++ b/packages/core/src/contexts/auth-context.tsx @@ -316,10 +316,7 @@ type AuthProviderProps = { * - initial deferred auto-login attempt * - duplicate login prevention */ -const useAutoLogin = (props: { - client: EnhancedAuthClient; - enabled?: boolean; -}) => { +const useAutoLogin = (props: { client: EnhancedAuthClient; enabled?: boolean }) => { // Prevent duplicate login redirects when multiple auth_state_changed // events fire before the first login attempt settles. const loginInFlightRef = useRef | null>(null); @@ -420,10 +417,7 @@ const useCallbackStatus = (client: EnhancedAuthClient) => { (notify: () => void) => client.subscribeCallbackStatus(notify), [client], ); - const getSnapshot = useCallback( - () => client.getCallbackStatusSnapshot(), - [client], - ); + const getSnapshot = useCallback(() => client.getCallbackStatusSnapshot(), [client]); return useSyncExternalStore(subscribe, getSnapshot); }; @@ -452,9 +446,7 @@ const useCallbackStatus = (client: EnhancedAuthClient) => { * } * ``` */ -export const AuthProvider = ( - props: React.PropsWithChildren, -) => { +export const AuthProvider = (props: React.PropsWithChildren) => { const client = props.client; // Set up auth state subscription for auto-login orchestration @@ -484,9 +476,7 @@ export const AuthProvider = ( // the callback settles. Guarded trees already wait on auth state instead. const callbackStatus = useCallbackStatus(client); const resolvedChildren = - callbackStatus === "pending" && props.guardComponent == null - ? null - : props.children; + callbackStatus === "pending" && props.guardComponent == null ? null : props.children; const authContextValue = useMemo( () => ({ @@ -502,9 +492,7 @@ export const AuthProvider = ( return ( {props.guardComponent ? ( - - {resolvedChildren} - + {resolvedChildren} ) : ( resolvedChildren )} @@ -518,9 +506,7 @@ export const AuthProvider = ( const useAuthContext = () => { const context = useContext(AuthContext); if (!context) { - throw new Error( - "useAuth/useAuthSuspense must be used within an AuthProvider", - ); + throw new Error("useAuth/useAuthSuspense must be used within an AuthProvider"); } return context; }; diff --git a/packages/core/src/routing/router.test.tsx b/packages/core/src/routing/router.test.tsx index 59dd14e2..afa313d0 100644 --- a/packages/core/src/routing/router.test.tsx +++ b/packages/core/src/routing/router.test.tsx @@ -1,17 +1,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { - act, - cleanup, - fireEvent, - render, - screen, - waitFor, -} from "@testing-library/react"; +import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { RouterContainer } from "./router"; -import { - AppShellConfigContext, - AppShellDataContext, -} from "@/contexts/appshell-context"; +import { AppShellConfigContext, AppShellDataContext } from "@/contexts/appshell-context"; import * as ReactRouter from "react-router"; import { Link, Outlet, useNavigate } from "react-router"; import { @@ -65,11 +55,7 @@ const renderWithConfig = ({ const tree = ( - + @@ -78,11 +64,7 @@ const renderWithConfig = ({ return render( authClient ? ( - + {tree} ) : ( @@ -232,9 +214,7 @@ describe("RouterContainer (memory)", () => { initialEntries: ["/dashboard"], }); - expect( - await screen.findByRole("alert", { name: "default-error-boundary" }), - ).toBeDefined(); + expect(await screen.findByRole("alert", { name: "default-error-boundary" })).toBeDefined(); cleanup(); @@ -243,9 +223,7 @@ describe("RouterContainer (memory)", () => { initialEntries: ["/dashboard/overview"], }); - expect( - await screen.findByRole("alert", { name: "default-error-boundary" }), - ).toBeDefined(); + expect(await screen.findByRole("alert", { name: "default-error-boundary" })).toBeDefined(); } finally { consoleSpy.mockRestore(); } @@ -257,9 +235,7 @@ describe("RouterContainer (memory)", () => { initialEntries: ["/unknown-path"], }); - expect( - await screen.findByRole("alert", { name: "default-error-boundary" }), - ).toBeDefined(); + expect(await screen.findByRole("alert", { name: "default-error-boundary" })).toBeDefined(); expect(await screen.findByText("404 Not Found")).toBeDefined(); }); @@ -390,9 +366,7 @@ describe("RouterContainer (memory)", () => { initialEntries: ["/admin"], }); - expect( - await screen.findByRole("alert", { name: "default-error-boundary" }), - ).toBeDefined(); + expect(await screen.findByRole("alert", { name: "default-error-boundary" })).toBeDefined(); expect(await screen.findByText("404 Not Found")).toBeDefined(); }); @@ -491,8 +465,7 @@ describe("RouterContainer with AuthProvider", () => { }); it("waits to render until callback handling finishes", async () => { - let callbackStatus: "idle" | "pending" | "resolved" | "rejected" = - "pending"; + let callbackStatus: "idle" | "pending" | "resolved" | "rejected" = "pending"; let callbackStatusListener: (() => void) | undefined; const authClient = createMockAuthClient( { isAuthenticated: true, error: null, isReady: true }, @@ -847,8 +820,7 @@ describe("RouterContainer with AuthProvider", () => { }); it("renders the correct page after OAuth callback with a specific path", async () => { - let callbackStatus: "idle" | "pending" | "resolved" | "rejected" = - "pending"; + let callbackStatus: "idle" | "pending" | "resolved" | "rejected" = "pending"; let callbackStatusListener: (() => void) | undefined; const authClient = createMockAuthClient( { isAuthenticated: true, error: null, isReady: true }, @@ -956,10 +928,7 @@ describe("RouterContainer with AuthProvider", () => { return (

Dashboard

- From 0120355ba152089bc75e1a28f7b87aeeff256c0b Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Sun, 19 Apr 2026 23:40:43 +0900 Subject: [PATCH 15/15] fix: stabilize useEnsureAuthInitialized on callback failure and clean up EnhancedAuthClient interface - useEnsureAuthInitialized now accepts callbackStatus as a parameter and only skips initialization while the callback is 'pending'; a 'rejected' status falls through so checkAuthStatus can still resolve the session - getCallbackStatusSnapshot and subscribeCallbackStatus made optional on EnhancedAuthClient so consumers who mock or implement the interface do not need to provide internal callback-tracking methods - useCallbackStatus uses optional chaining with 'idle' fallback for clients that do not supply the optional methods - update createAuthClient JSDoc to document the OAuth callback side effect - update guardComponent JSDoc to reflect that AuthProvider renders the guard directly (no router dependency) - add tests: rejected callback triggers checkAuthStatus, createAuthClient calls handleCallback on OAuth URL and skips it on plain URLs --- .../core/src/contexts/auth-context.test.tsx | 88 +++++++++++++++++-- packages/core/src/contexts/auth-context.tsx | 48 ++++++---- 2 files changed, 111 insertions(+), 25 deletions(-) diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx index 6ffff7fd..f77e69b5 100644 --- a/packages/core/src/contexts/auth-context.test.tsx +++ b/packages/core/src/contexts/auth-context.test.tsx @@ -9,12 +9,14 @@ vi.mock("@tailor-platform/auth-public-client", () => ({ })); import { + createAuthClient, AuthProvider, useAuth, useEnsureAuthInitialized, useAuthSuspense, type EnhancedAuthClient, } from "./auth-context"; +import { createAuthClient as createAuthClientMock } from "@tailor-platform/auth-public-client"; afterEach(() => { cleanup(); @@ -77,8 +79,6 @@ describe("AuthProvider", () => { getAuthHeaders: vi.fn(), fetch: vi.fn(), getAppUri: vi.fn(() => "https://api.test.com"), - getCallbackStatusSnapshot: vi.fn(() => "idle"), - subscribeCallbackStatus: vi.fn(() => () => {}), ...otherOverrides, handleCallback, } as EnhancedAuthClient; @@ -101,7 +101,7 @@ describe("AuthProvider", () => { checkAuthStatus: mockCheckAuthStatus, }); - const { result } = renderHook(() => useEnsureAuthInitialized(mockClient)); + const { result } = renderHook(() => useEnsureAuthInitialized(mockClient, "idle")); await act(async () => { await result.current(); @@ -138,7 +138,7 @@ describe("AuthProvider", () => { checkAuthStatus: mockCheckAuthStatus, }); - const { result } = renderHook(() => useEnsureAuthInitialized(mockClient)); + const { result } = renderHook(() => useEnsureAuthInitialized(mockClient, "idle")); const mountRetry = result.current(); @@ -173,7 +173,7 @@ describe("AuthProvider", () => { checkAuthStatus: mockCheckAuthStatus, }); - const { result } = renderHook(() => useEnsureAuthInitialized(mockClient)); + const { result } = renderHook(() => useEnsureAuthInitialized(mockClient, "pending")); await act(async () => { await result.current(); @@ -181,6 +181,34 @@ describe("AuthProvider", () => { expect(mockCheckAuthStatus).not.toHaveBeenCalled(); }); + + it("should run auth initialization after callback is rejected", async () => { + window.history.replaceState({}, "", "/?code=auth-code-123&state=abc"); + + const state = { + isAuthenticated: false, + error: null, + isReady: false, + }; + const mockCheckAuthStatus = vi.fn().mockResolvedValue({ + isAuthenticated: false, + error: null, + isReady: true, + }); + + const mockClient = createMockAuthClient(state, { + checkAuthStatus: mockCheckAuthStatus, + }); + + // callbackStatus "rejected" means handleCallback failed — initialization should proceed + const { result } = renderHook(() => useEnsureAuthInitialized(mockClient, "rejected")); + + await act(async () => { + await result.current(); + }); + + expect(mockCheckAuthStatus).toHaveBeenCalledTimes(1); + }); }); describe("initial state", () => { @@ -327,15 +355,13 @@ describe("AuthProvider", () => { }); it("should render guarded children while callback is pending", async () => { - const pendingCallbackStatus: ReturnType = - "pending"; const state = { isAuthenticated: true, error: null, isReady: true, }; const mockClient = createMockAuthClient(state, { - getCallbackStatusSnapshot: vi.fn(() => pendingCallbackStatus), + getCallbackStatusSnapshot: vi.fn(() => "pending" as const), }); render( @@ -895,3 +921,49 @@ describe("AuthProvider", () => { }); }); }); + +describe("createAuthClient", () => { + const makeBaseClient = (mockHandleCallback: ReturnType) => ({ + handleCallback: mockHandleCallback, + getState: vi.fn(() => ({ + isAuthenticated: false, + error: null, + isReady: false, + })), + login: vi.fn(), + logout: vi.fn(), + getAuthUrl: vi.fn(), + checkAuthStatus: vi.fn(), + refreshTokens: vi.fn(), + ready: vi.fn(() => Promise.resolve()), + configure: vi.fn(), + addEventListener: vi.fn(() => () => {}), + getAuthHeaders: vi.fn(), + fetch: vi.fn(), + getAuthHeadersForQuery: vi.fn(), + }); + + it("calls handleCallback immediately when URL contains OAuth callback parameters", () => { + window.history.replaceState({}, "", "/?code=auth-code-123"); + + const mockHandleCallback = vi.fn().mockResolvedValue(undefined); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + vi.mocked(createAuthClientMock).mockReturnValue(makeBaseClient(mockHandleCallback) as any); + + createAuthClient({ clientId: "test", appUri: "https://test.com" }); + + expect(mockHandleCallback).toHaveBeenCalledTimes(1); + }); + + it("does not call handleCallback when URL has no OAuth parameters", () => { + // URL is already "/" from afterEach reset + + const mockHandleCallback = vi.fn().mockResolvedValue(undefined); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + vi.mocked(createAuthClientMock).mockReturnValue(makeBaseClient(mockHandleCallback) as any); + + createAuthClient({ clientId: "test", appUri: "https://test.com" }); + + expect(mockHandleCallback).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx index 61db4a4e..2cb86b7d 100644 --- a/packages/core/src/contexts/auth-context.tsx +++ b/packages/core/src/contexts/auth-context.tsx @@ -50,15 +50,17 @@ export interface EnhancedAuthClient extends AuthClient { /** * Returns the current OAuth callback handling state. + * Provided automatically by clients created with {@link createAuthClient}. * @internal */ - getCallbackStatusSnapshot(): CallbackStatus; + getCallbackStatusSnapshot?(): CallbackStatus; /** * Subscribe to callback settlement changes. + * Provided automatically by clients created with {@link createAuthClient}. * @internal */ - subscribeCallbackStatus(listener: () => void): () => void; + subscribeCallbackStatus?(listener: () => void): () => void; } /** @@ -113,6 +115,11 @@ const createCallbackStatusManager = () => { * This wrapper around the original createAuthClient adds convenience methods * to reduce duplication of the appUri across your application. * + * **Side effect on call**: If the current URL contains OAuth callback parameters + * (`code` or `error`), this function immediately starts `handleCallback()` to + * exchange the authorization code. This happens at call time — before any React + * render — so it is safe to call at module scope. + * * @example * ```tsx * import { createAuthClient, AuthProvider } from '@tailor-platform/app-shell'; @@ -297,13 +304,11 @@ type AuthProviderProps = { /** * Guard UI component to show when loading or unauthenticated. * - * If not provided, children will be just rendered in any auth state. + * When provided, AuthProvider renders this component directly while auth + * is not ready or the user is not authenticated. Children are hidden until + * auth resolves to an authenticated state. * - * Note: This prop only takes effect when AuthProvider wraps an AppShell - * component. The guard is rendered by the router's root route element, - * so it requires RouterContainer to be present in the component tree. - * For guarding components placed between AuthProvider and AppShell, - * rely on the authState from useAuth() directly. + * If not provided, children are rendered regardless of auth state. */ guardComponent?: () => React.ReactNode; }; @@ -376,14 +381,19 @@ const useAutoLogin = (props: { client: EnhancedAuthClient; enabled?: boolean }) * initialization flow stays visible at the call site, while overlapping * checks still collapse into a single request. */ -export const useEnsureAuthInitialized = (client: EnhancedAuthClient) => { +export const useEnsureAuthInitialized = ( + client: EnhancedAuthClient, + callbackStatus: CallbackStatus, +) => { const initInFlightRef = useRef | null>(null); const ensureInitialized = useCallback(async (): Promise => { const authState = client.getState(); - const isCallbackUrl = isCurrentOAuthCallbackUrl(); - if (isCallbackUrl || authState.isReady) { + // Skip initialization while a callback exchange is actively in progress. + // If the callback fails ("rejected"), we fall through so checkAuthStatus + // can still resolve the session instead of leaving isReady permanently false. + if (callbackStatus === "pending" || authState.isReady) { return; } @@ -402,7 +412,7 @@ export const useEnsureAuthInitialized = (client: EnhancedAuthClient) => { }); return initInFlightRef.current; - }, [client]); + }, [client, callbackStatus]); return ensureInitialized; }; @@ -412,12 +422,12 @@ export const useEnsureAuthInitialized = (client: EnhancedAuthClient) => { * useSyncExternalStore so AuthProvider can coordinate rendering while the * callback exchange is still in flight. */ -const useCallbackStatus = (client: EnhancedAuthClient) => { +const useCallbackStatus = (client: EnhancedAuthClient): CallbackStatus => { const subscribe = useCallback( - (notify: () => void) => client.subscribeCallbackStatus(notify), + (notify: () => void) => client.subscribeCallbackStatus?.(notify) ?? (() => {}), [client], ); - const getSnapshot = useCallback(() => client.getCallbackStatusSnapshot(), [client]); + const getSnapshot = useCallback(() => client.getCallbackStatusSnapshot?.() ?? "idle", [client]); return useSyncExternalStore(subscribe, getSnapshot); }; @@ -459,9 +469,14 @@ export const AuthProvider = (props: React.PropsWithChildren) const getSnapshot = useCallback(() => client.getState(), [client]); const authState = useSyncExternalStore(subscribeAuthState, getSnapshot); + // Read callback status first so it can be passed to useEnsureAuthInitialized. + // This lets initialization retry automatically when a pending callback settles + // to rejected — the new ensureInitialized reference triggers the useEffect again. + const callbackStatus = useCallbackStatus(client); + // Prepare a shared initialization function so AuthProvider can start the // first auth check itself without depending on router navigation. - const ensureAuthInitialized = useEnsureAuthInitialized(client); + const ensureAuthInitialized = useEnsureAuthInitialized(client, callbackStatus); // AuthProvider owns the normal startup path: on mount, ask the auth client // to resolve the current session so consumers can rely on authState even @@ -474,7 +489,6 @@ export const AuthProvider = (props: React.PropsWithChildren) // While handling an OAuth callback, keep unguarded children hidden until // the callback settles. Guarded trees already wait on auth state instead. - const callbackStatus = useCallbackStatus(client); const resolvedChildren = callbackStatus === "pending" && props.guardComponent == null ? null : props.children;