diff --git a/.gitignore b/.gitignore index 964ef6d4..f0bd5dca 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,5 @@ results .env .claude/settings.json + +next-env.d.ts diff --git a/packages/example-nextjs16/next-env.d.ts b/packages/example-nextjs16/next-env.d.ts deleted file mode 100644 index ce4e94a6..00000000 --- a/packages/example-nextjs16/next-env.d.ts +++ /dev/null @@ -1,7 +0,0 @@ -/// -/// -import "./.next/types/routes.d.ts"; -import "./.next/types/root-params.d.ts"; - -// NOTE: This file should not be edited -// see https://nextjs.org/docs/app/api-reference/config/typescript for more information. diff --git a/packages/urlstate/useInsertionEffect.ts b/packages/urlstate/useInsertionEffect.ts deleted file mode 100644 index 25eb24c2..00000000 --- a/packages/urlstate/useInsertionEffect.ts +++ /dev/null @@ -1,3 +0,0 @@ -import React from "react"; - -export const useInsertionEffect = React?.useInsertionEffect || React.useEffect; diff --git a/packages/urlstate/useIsomorphicLayoutEffect.ts b/packages/urlstate/useIsomorphicLayoutEffect.ts new file mode 100644 index 00000000..8a8563a9 --- /dev/null +++ b/packages/urlstate/useIsomorphicLayoutEffect.ts @@ -0,0 +1,5 @@ +import React from 'react'; + +// subscribe before paint; useLayoutEffect warns on the server +export const useIsomorphicLayoutEffect = + typeof window === 'undefined' ? React.useEffect : React.useLayoutEffect; diff --git a/packages/urlstate/useSharedState/useSharedState.test.ts b/packages/urlstate/useSharedState/useSharedState.test.ts index 2caf1d36..81ef97be 100644 --- a/packages/urlstate/useSharedState/useSharedState.test.ts +++ b/packages/urlstate/useSharedState/useSharedState.test.ts @@ -1,3 +1,4 @@ +import React from 'react'; import { act, renderHook } from '@testing-library/react'; import { useSharedState } from './useSharedState'; @@ -200,6 +201,54 @@ describe('useSharedState', () => { expect(mockSubscriber).toHaveBeenCalledTimes(1); }); + test('should catch a store write that lands between render and subscribe', () => { + const defaultState = { count: 0 }; + const updated = { count: 7 }; + // renders after the hook, so the write lands before it subscribes + const WriteMidRender = () => { + subscribers.stateMap.set(defaultState, updated); + return null; + }; + + const { result } = renderHook(() => useSharedState(defaultState), { + wrapper: ({ children }) => + React.createElement( + React.Fragment, + null, + children, + React.createElement(WriteMidRender), + ), + }); + + expect(result.current.state).toStrictEqual(updated); + }); + + test('should subscribe before paint, not after', () => { + const defaultState = { count: 0 }; + const order: string[] = []; + vi.spyOn(subscribers.subscribers, 'add').mockImplementation(() => { + order.push('subscribe'); + return () => void 0; + }); + const Probe = () => { + React.useLayoutEffect(() => void order.push('layout'), []); + React.useEffect(() => void order.push('effect'), []); + return null; + }; + + renderHook(() => useSharedState(defaultState), { + wrapper: ({ children }) => + React.createElement( + React.Fragment, + null, + children, + React.createElement(Probe), + ), + }); + + expect(order).toStrictEqual(['subscribe', 'layout', 'effect']); + }); + describe('few instances', () => { test('should set initial value only 1 time', () => { const defaultState = { count: 0 }; diff --git a/packages/urlstate/useSharedState/useSharedState.ts b/packages/urlstate/useSharedState/useSharedState.ts index 22c7c071..9e95930a 100644 --- a/packages/urlstate/useSharedState/useSharedState.ts +++ b/packages/urlstate/useSharedState/useSharedState.ts @@ -1,7 +1,7 @@ import React from 'react'; import { stateMap, subscribers } from '../subscribers'; -import { useInsertionEffect } from '../useInsertionEffect'; +import { useIsomorphicLayoutEffect } from '../useIsomorphicLayoutEffect'; import { isEqual, isSSR, type JSONCompatible } from '../utils'; /** @@ -65,18 +65,16 @@ export function useSharedState( [], ); - useInsertionEffect(() => { + useIsomorphicLayoutEffect(() => { const cb = () => { _setState(stateMap.get(stateShape.current) || stateShape.current); }; - return subscribers.add(stateShape.current, cb); - }, []); + const unsubscribe = subscribers.add(stateShape.current, cb); - // stateMap can change between render and subscribing; that write skips this - // instance, and later ones compare against the map, so it never catches up. - // Same object back when nothing changed, React bails out on identity. - React.useEffect(() => { + // catch a write that landed between render and subscribing _setState((curr) => stateMap.get(stateShape.current) ?? curr); + + return unsubscribe; }, []); // get state without deps diff --git a/packages/urlstate/useUrlStateBase/useUrlStateBase.test.ts b/packages/urlstate/useUrlStateBase/useUrlStateBase.test.ts index 142b02e4..5e1e0f87 100644 --- a/packages/urlstate/useUrlStateBase/useUrlStateBase.test.ts +++ b/packages/urlstate/useUrlStateBase/useUrlStateBase.test.ts @@ -1,3 +1,4 @@ +import React from 'react'; import { act, fireEvent, renderHook } from '@testing-library/react'; // import { advanceTimersByTime } from '../../../tests/testUtils'; @@ -556,17 +557,12 @@ describe('useUrlStateBase', () => { describe('back/forward history navigation', () => { test('should update state on back/forward', () => { vi.mocked(utils).isSSR = false; - const search = '?num=55'; const originalLocation = window.location; - vi - .spyOn(window, 'location', 'get') - .mockImplementationOnce(() => ({ - ...originalLocation, - })) - .mockImplementationOnce(() => ({ - ...originalLocation, - search, - })); + let search = ''; + vi.spyOn(window, 'location', 'get').mockImplementation(() => ({ + ...originalLocation, + search, + })); const { result } = renderHook(() => useUrlStateBase(shape, router, ({ parse }) => parse(window.location.search), @@ -575,11 +571,72 @@ describe('useUrlStateBase', () => { expect(result.current.state).toStrictEqual(shape); + search = '?num=55'; act(() => { fireEvent.popState(window); }); expect(result.current.state).toStrictEqual({ ...shape, num: 55 }); }); + + test('should pick up a popstate that fired before the listener registered', () => { + vi.mocked(utils).isSSR = false; + const originalLocation = window.location; + let search = ''; + vi.spyOn(window, 'location', 'get').mockImplementation(() => ({ + ...originalLocation, + search, + })); + + // renders after the hook, so the url moves before it listens + const UrlChangedMidRender = () => { + search = '?num=55'; + return null; + }; + const { result } = renderHook( + () => + useUrlStateBase(shape, router, ({ parse }) => + parse(window.location.search), + ), + { + wrapper: ({ children }) => + React.createElement( + React.Fragment, + null, + children, + React.createElement(UrlChangedMidRender), + ), + }, + ); + + expect(result.current.state).toStrictEqual({ ...shape, num: 55 }); + }); + + test('should keep an updateState-only value when the url did not move', () => { + vi.mocked(utils).isSSR = false; + const originalLocation = window.location; + vi.spyOn(window, 'location', 'get').mockImplementation(() => ({ + ...originalLocation, + search: '', + })); + const { result } = renderHook(() => + useUrlStateBase(shape, router, ({ parse }) => + parse(window.location.search), + ), + ); + + act(() => { + result.current.updateState({ num: 55 }); + }); + + const second = renderHook(() => + useUrlStateBase(shape, router, ({ parse }) => + parse(window.location.search), + ), + ); + + expect(second.result.current.state).toStrictEqual({ ...shape, num: 55 }); + expect(result.current.state).toStrictEqual({ ...shape, num: 55 }); + }); }); describe('basename prop', () => { diff --git a/packages/urlstate/useUrlStateBase/useUrlStateBase.ts b/packages/urlstate/useUrlStateBase/useUrlStateBase.ts index b06c299e..90d00a14 100644 --- a/packages/urlstate/useUrlStateBase/useUrlStateBase.ts +++ b/packages/urlstate/useUrlStateBase/useUrlStateBase.ts @@ -1,7 +1,7 @@ import React from 'react'; import { TIMEOUT } from '../constants'; -import { useInsertionEffect } from '../useInsertionEffect'; +import { useIsomorphicLayoutEffect } from '../useIsomorphicLayoutEffect'; import { useSharedState } from '../useSharedState'; import { useUrlEncode } from '../useUrlEncode'; import { @@ -78,7 +78,11 @@ export function useUrlStateBase( }) || defaultState, ); - useInsertionEffect(() => { + const searchAtRender = React.useRef(); + if (searchAtRender.current === undefined) + searchAtRender.current = getSearch(); + + useIsomorphicLayoutEffect(() => { // for history navigation const popCb = () => { setState(parse(filterUnknownParamsClient(defaultState, getSearch()))); @@ -86,6 +90,9 @@ export function useUrlStateBase( window.addEventListener(popstateEv, popCb); + // popstate before registration is not replayed + if (searchAtRender.current !== getSearch()) popCb(); + return () => { window.removeEventListener(popstateEv, popCb); }; diff --git a/packages/urlstate/utils.test.ts b/packages/urlstate/utils.test.ts index e1233663..d8beefa8 100644 --- a/packages/urlstate/utils.test.ts +++ b/packages/urlstate/utils.test.ts @@ -1,4 +1,4 @@ -import { getParams, typeOf, assignValue, filterUnknownParamsClient, filterUnknown, filterUnknownParams, isPrimitive, getSearch } from './utils'; +import { getParams, typeOf, assignValue, filterUnknownParamsClient, filterUnknown, filterUnknownParams, isPrimitive, getSearch, subscribeToUrl } from './utils'; describe('typeOf', () => { test('string', () => { @@ -391,3 +391,33 @@ describe('getSearch', () => { }); }); }); + +describe('subscribeToUrl', () => { + test('notifies after the pushState call returns, not inside it', async () => { + const cb = vi.fn(); + const originalHref = window.location.href; + const unsubscribe = subscribeToUrl(cb); + + try { + window.history.pushState(null, '', '/sub?a=1'); + expect(cb).not.toHaveBeenCalled(); + + await Promise.resolve(); + expect(cb).toHaveBeenCalledTimes(1); + + window.history.replaceState(null, '', '/sub?a=2'); + expect(cb).toHaveBeenCalledTimes(1); + + await Promise.resolve(); + expect(cb).toHaveBeenCalledTimes(2); + + unsubscribe(); + window.history.pushState(null, '', '/sub?a=3'); + await Promise.resolve(); + expect(cb).toHaveBeenCalledTimes(2); + } finally { + unsubscribe(); + window.history.replaceState(null, '', originalHref); + } + }); +}); diff --git a/packages/urlstate/utils.ts b/packages/urlstate/utils.ts index afd93b17..1cc4ee22 100644 --- a/packages/urlstate/utils.ts +++ b/packages/urlstate/utils.ts @@ -243,8 +243,11 @@ function patchHistory() { ...args: Parameters ) { const result = original.apply(this, args); - // copy, a listener can unsubscribe while iterating - for (const cb of [...urlListeners]) cb(); + // Next writes history from a useInsertionEffect, which must not schedule updates + queueMicrotask(() => { + // copy, a listener can unsubscribe while iterating + for (const cb of [...urlListeners]) cb(); + }); return result; }; }