From 4e06c6489ed56e9b9459b0775e3e375ff0dde662 Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Fri, 17 Apr 2026 16:09:48 +0900 Subject: [PATCH 1/9] 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 2/9] 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 3/9] 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 4/9] 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 5/9] 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 6/9] 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 7/9] 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 8/9] 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 1d16c644863060b553c0e5ee5eaa07afa3a15169 Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Sat, 18 Apr 2026 11:56:47 +0900 Subject: [PATCH 9/9] Stabilize router memo inputs --- packages/core/src/routing/router.test.tsx | 109 ++++++++++++++++++++++ packages/core/src/routing/router.tsx | 47 ++++++---- 2 files changed, 137 insertions(+), 19 deletions(-) diff --git a/packages/core/src/routing/router.test.tsx b/packages/core/src/routing/router.test.tsx index 7375b694..ce368cf4 100644 --- a/packages/core/src/routing/router.test.tsx +++ b/packages/core/src/routing/router.test.tsx @@ -20,6 +20,8 @@ afterEach(() => { cleanup(); }); +const homeRootComponent = () =>
Home
; + const renderWithConfig = ({ modules = [], basePath, @@ -537,6 +539,113 @@ describe("RouterContainer with AuthProvider", () => { } }); + it("does not recreate the router when shell children change", async () => { + const createMemoryRouterSpy = vi.spyOn(ReactRouter, "createMemoryRouter"); + const initialEntries = ["/"]; + const configurations = { + modules: [], + settingsResources: [] as Array, + basePath: undefined, + errorBoundary: undefined, + locale: "en", + }; + + try { + const view = render( + + + +
+ Shell A + +
+
+
+
, + ); + + expect(await screen.findByText("Shell A")).toBeDefined(); + expect(await screen.findByText("Home")).toBeDefined(); + expect(createMemoryRouterSpy).toHaveBeenCalledTimes(1); + + view.rerender( + + + +
+ Shell B + +
+
+
+
, + ); + + expect(await screen.findByText("Shell B")).toBeDefined(); + expect(screen.queryByText("Shell A")).toBeNull(); + expect(await screen.findByText("Home")).toBeDefined(); + expect(createMemoryRouterSpy).toHaveBeenCalledTimes(1); + } finally { + createMemoryRouterSpy.mockRestore(); + } + }); + + it("does not recreate the router when guard wrapper changes", async () => { + const createMemoryRouterSpy = vi.spyOn(ReactRouter, "createMemoryRouter"); + const initialEntries = ["/"]; + const authClient = createMockAuthClient({ + isAuthenticated: false, + error: null, + isReady: true, + }); + const configurations = { + modules: [], + settingsResources: [] as Array, + basePath: undefined, + errorBoundary: undefined, + locale: "en", + }; + + try { + const tree = (guardComponent: () => React.ReactNode) => ( + + + + + + + + + + ); + + const view = render(tree(() =>
Guard A
)); + + expect(await screen.findByText("Guard A")).toBeDefined(); + expect(createMemoryRouterSpy).toHaveBeenCalledTimes(1); + + view.rerender(tree(() =>
Guard B
)); + + expect(await screen.findByText("Guard B")).toBeDefined(); + expect(screen.queryByText("Guard A")).toBeNull(); + 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({ diff --git a/packages/core/src/routing/router.tsx b/packages/core/src/routing/router.tsx index d5025684..9e3c6166 100644 --- a/packages/core/src/routing/router.tsx +++ b/packages/core/src/routing/router.tsx @@ -1,11 +1,23 @@ -import { type PropsWithChildren, type ReactNode, useMemo } from "react"; +import { createContext, type PropsWithChildren, type ReactNode, useContext, useMemo } from "react"; import { Outlet, createMemoryRouter, createBrowserRouter, RouterProvider } from "react-router"; import type { LoaderFunctionArgs, 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"; +import { useRootRouteContext, type RootRouteLoaderFn } from "@/contexts/root-route-context"; + +// The router should only be recreated when route-defining inputs change. +// The shell element and auth guard wrapper are render-time concerns, so keep +// them in a separate React context instead of baking them into the route tree. +const RootRouteContext = createContext(null); + +const RootRouteElement = () => { + const shell = useContext(RootRouteContext); + const rootRouteCtx = useRootRouteContext(); + + return rootRouteCtx?.wrapComponent ? rootRouteCtx.wrapComponent(shell) : shell; +}; // ============================================================================ // Root Route @@ -15,21 +27,18 @@ import { useRootRouteContext, type RootRouteContextType } from "@/contexts/root- * 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. + * When AuthProvider wraps AppShell, rootLoader runs before rendering + * (e.g. OAuth callback handling). The element wrapper stays outside the + * route definition so UI-only rerenders do not force router recreation. */ const createRootRoute = (params: { configurations: RootConfiguration; - rootRouteCtx: RootRouteContextType | null; + rootLoader: RootRouteLoaderFn | null; contentRoutes: Array; - children: ReactNode; }): RouteObject => { - const { configurations, rootRouteCtx, contentRoutes, children } = params; + const { configurations, rootLoader, contentRoutes } = params; // --- Loader: combine auth callback handling with navigation loading --- - const rootLoader = rootRouteCtx?.loader ?? null; const { loaderID, loader: navLoader } = createNavItemsLoader({ modules: configurations.modules, locale: configurations.locale, @@ -42,10 +51,6 @@ const createRootRoute = (params: { 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; const routeChildren = globalErrorBoundary @@ -61,7 +66,7 @@ const createRootRoute = (params: { return { id: loaderID, loader, - element, + element: , children: routeChildren, // Hydration fallback is unused in CSR-only usage of AppShell. // Return null to silence hydration warnings. @@ -91,6 +96,7 @@ export const RouterContainer = (props: PropsWithChildren) const { rootComponent, children } = props; const { configurations } = useAppShellConfig(); const rootRouteCtx = useRootRouteContext(); + const rootLoader = rootRouteCtx?.loader ?? null; const contentRoutes = useMemo( () => createContentRoutes({ @@ -106,12 +112,11 @@ export const RouterContainer = (props: PropsWithChildren) [ createRootRoute({ configurations, - rootRouteCtx, + rootLoader, contentRoutes, - children, }), ] satisfies Array, - [configurations, rootRouteCtx, contentRoutes, children], + [configurations, rootLoader, contentRoutes], ); const basename = configurations.basePath ? "/" + configurations.basePath : undefined; @@ -134,5 +139,9 @@ export const RouterContainer = (props: PropsWithChildren) [basename, initialEntries, props.memory, routes], ); - return ; + return ( + + + + ); };