mirror of
https://github.com/nexu-io/open-design.git
synced 2026-06-01 03:14:35 +07:00
fix(web/router): defer popstate dispatch to microtask (#2490)
* fix(web/router): defer popstate dispatch to microtask navigate() previously dispatched a synchronous popstate event after mutating window.history, which caused React 18 to emit: Cannot update a component (Router) while rendering a different component (App). To locate the bad setState() call inside App, follow the stack trace as described in https://react.dev/link/setstate-in-render This happens whenever a caller invokes navigate() from inside a useState updater (e.g. App.tsx:479 routing first-run users through the onboarding panel from inside the setConfig() update). The synchronous popstate dispatch reaches useRoute() subscribers which then call setRoute() while the parent component is still rendering. Defer the popstate dispatch to a microtask. The window.history call itself stays synchronous so the URL bar updates immediately; only subscriber updates are pushed past the current render commit, which removes the warning without changing observable behaviour for any existing caller. * fix(web/router): cover deferred navigation timing --------- Co-authored-by: Visionboost <contact@visionboost.fr> Co-authored-by: Siri-Ray <2667192167@qq.com>
This commit is contained in:
parent
e71938767e
commit
0c4b7e50be
2 changed files with 109 additions and 9 deletions
|
|
@ -3,7 +3,7 @@
|
|||
// we want a single source of truth for "what file is open" — encoding
|
||||
// that in the URL is the simplest way to make it deep-linkable.
|
||||
|
||||
import { useEffect, useState } from 'react';
|
||||
import { useSyncExternalStore } from 'react';
|
||||
|
||||
// Entry-shell sub-views. The home/project landing renders one of three
|
||||
// columns and each sub-view now owns a top-level path so the browser
|
||||
|
|
@ -137,6 +137,14 @@ export function buildPath(route: Route): string {
|
|||
// Centralized navigation. Components call this instead of mutating
|
||||
// `window.location` directly so we can fan the change out to any
|
||||
// `useRoute()` subscriber via a custom event.
|
||||
//
|
||||
// The `popstate` dispatch is deferred to a microtask so that callers
|
||||
// can safely invoke `navigate()` from inside a `useState` updater or
|
||||
// during a render commit phase without triggering React's
|
||||
// "Cannot update a component while rendering a different component"
|
||||
// warning. The `history` API call itself stays synchronous so the URL
|
||||
// bar updates immediately; only the `useRoute()` subscriber updates
|
||||
// are deferred past the current render.
|
||||
export function navigate(route: Route, opts: { replace?: boolean } = {}): void {
|
||||
const target = buildPath(route);
|
||||
const current = window.location.pathname;
|
||||
|
|
@ -146,15 +154,28 @@ export function navigate(route: Route, opts: { replace?: boolean } = {}): void {
|
|||
} else {
|
||||
window.history.pushState(null, '', target);
|
||||
}
|
||||
window.dispatchEvent(new PopStateEvent('popstate'));
|
||||
queueMicrotask(() => {
|
||||
window.dispatchEvent(new PopStateEvent('popstate'));
|
||||
});
|
||||
}
|
||||
|
||||
let cachedPathname: string | null = null;
|
||||
let cachedRoute: Route | null = null;
|
||||
|
||||
function getRouteSnapshot(): Route {
|
||||
const pathname = window.location.pathname;
|
||||
if (cachedPathname !== pathname || cachedRoute === null) {
|
||||
cachedPathname = pathname;
|
||||
cachedRoute = parseRoute(pathname);
|
||||
}
|
||||
return cachedRoute;
|
||||
}
|
||||
|
||||
function subscribeToRouteChanges(onStoreChange: () => void): () => void {
|
||||
window.addEventListener('popstate', onStoreChange);
|
||||
return () => window.removeEventListener('popstate', onStoreChange);
|
||||
}
|
||||
|
||||
export function useRoute(): Route {
|
||||
const [route, setRoute] = useState<Route>(() => parseRoute(window.location.pathname));
|
||||
useEffect(() => {
|
||||
const onPop = () => setRoute(parseRoute(window.location.pathname));
|
||||
window.addEventListener('popstate', onPop);
|
||||
return () => window.removeEventListener('popstate', onPop);
|
||||
}, []);
|
||||
return route;
|
||||
return useSyncExternalStore(subscribeToRouteChanges, getRouteSnapshot, getRouteSnapshot);
|
||||
}
|
||||
|
|
|
|||
79
apps/web/tests/router.navigate.test.tsx
Normal file
79
apps/web/tests/router.navigate.test.tsx
Normal file
|
|
@ -0,0 +1,79 @@
|
|||
// @vitest-environment jsdom
|
||||
|
||||
import { act, cleanup, render, screen, waitFor } from '@testing-library/react';
|
||||
import { useEffect, useState } from 'react';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { navigate, useRoute } from '../src/router';
|
||||
|
||||
function RouteLabel() {
|
||||
const route = useRoute();
|
||||
const label = route.kind === 'home' ? route.view : route.kind;
|
||||
return <div data-testid="route-label">{label}</div>;
|
||||
}
|
||||
|
||||
function NavigateFromUpdater() {
|
||||
const [didNavigate, setDidNavigate] = useState(false);
|
||||
|
||||
useEffect(() => {
|
||||
if (didNavigate) return;
|
||||
setDidNavigate(() => {
|
||||
navigate({ kind: 'home', view: 'onboarding' }, { replace: true });
|
||||
return true;
|
||||
});
|
||||
}, [didNavigate]);
|
||||
|
||||
return <RouteLabel />;
|
||||
}
|
||||
|
||||
async function flushMicrotasks() {
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
});
|
||||
}
|
||||
|
||||
describe('navigate / useRoute timing', () => {
|
||||
let consoleError: ReturnType<typeof vi.spyOn>;
|
||||
|
||||
beforeEach(() => {
|
||||
window.history.replaceState(null, '', '/');
|
||||
consoleError = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
consoleError.mockRestore();
|
||||
window.history.replaceState(null, '', '/');
|
||||
});
|
||||
|
||||
it('updates history synchronously and notifies listeners after the microtask boundary', async () => {
|
||||
const onPop = vi.fn();
|
||||
window.addEventListener('popstate', onPop);
|
||||
|
||||
navigate({ kind: 'home', view: 'onboarding' }, { replace: true });
|
||||
|
||||
expect(window.location.pathname).toBe('/onboarding');
|
||||
expect(onPop).not.toHaveBeenCalled();
|
||||
|
||||
await flushMicrotasks();
|
||||
|
||||
expect(onPop).toHaveBeenCalledTimes(1);
|
||||
window.removeEventListener('popstate', onPop);
|
||||
});
|
||||
|
||||
it('updates route subscribers after render-phase updater navigation without React warnings', async () => {
|
||||
render(<NavigateFromUpdater />);
|
||||
|
||||
await flushMicrotasks();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId('route-label').textContent).toBe('onboarding');
|
||||
});
|
||||
expect(window.location.pathname).toBe('/onboarding');
|
||||
|
||||
const warningCalls = consoleError.mock.calls.filter((call: unknown[]) =>
|
||||
String(call[0]).includes('Cannot update a component'),
|
||||
);
|
||||
expect(warningCalls).toEqual([]);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue