From 9fffcfaff85ffef96b1a4214277551f6f644642a Mon Sep 17 00:00:00 2001 From: Francois Best Date: Wed, 28 Aug 2024 16:35:17 +0200 Subject: [PATCH 1/4] test: Making sure it fails beforehand --- .../cypress/e2e/referential-equality.cy.js | 29 ++++++ .../src/app/app/referential-equality/page.tsx | 98 +++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 packages/e2e/cypress/e2e/referential-equality.cy.js create mode 100644 packages/e2e/src/app/app/referential-equality/page.tsx diff --git a/packages/e2e/cypress/e2e/referential-equality.cy.js b/packages/e2e/cypress/e2e/referential-equality.cy.js new file mode 100644 index 000000000..f5a068937 --- /dev/null +++ b/packages/e2e/cypress/e2e/referential-equality.cy.js @@ -0,0 +1,29 @@ +/// + +it('Referential equality', () => { + cy.visit('/app/referential-equality') + cy.contains('#hydration-marker', 'hydrated').should('be.hidden') + cy.get('#ref-a').should('have.text', '1') + cy.get('#ref-b').should('have.text', '1') + cy.get('#increment-a').click() + cy.get('#ref-a').should('have.text', '2') + cy.get('#ref-b').should('have.text', '1') + cy.get('#increment-b').click() + cy.get('#ref-a').should('have.text', '2') + cy.get('#ref-b').should('have.text', '2') + cy.get('#idempotent-a').click() + cy.get('#ref-a').should('have.text', '2') + cy.get('#ref-b').should('have.text', '2') + cy.get('#idempotent-b').click() + cy.get('#ref-a').should('have.text', '2') + cy.get('#ref-b').should('have.text', '2') + cy.get('#clear-a').click() + cy.get('#ref-a').should('have.text', '3') + cy.get('#ref-b').should('have.text', '2') + cy.get('#clear-b').click() + cy.get('#ref-a').should('have.text', '3') + cy.get('#ref-b').should('have.text', '3') + cy.get('#link').click() + cy.get('#ref-a').should('have.text', '3') + cy.get('#ref-b').should('have.text', '3') +}) diff --git a/packages/e2e/src/app/app/referential-equality/page.tsx b/packages/e2e/src/app/app/referential-equality/page.tsx new file mode 100644 index 000000000..e6ca0accf --- /dev/null +++ b/packages/e2e/src/app/app/referential-equality/page.tsx @@ -0,0 +1,98 @@ +'use client' + +import Link from 'next/link' +import { parseAsJson, useQueryState, useQueryStates } from 'nuqs' +import { Suspense, useEffect, useState } from 'react' + +export default function Page() { + return ( + + + + ) +} + +const defaultValue = { x: 0 } +type Value = typeof defaultValue + +function increment(value: Value): Value { + return { x: value.x + 1 } +} + +const makeLoggingSpy = + (key: string) => + (value: unknown): Value => { + console.log(`[%s]: Parser running with value %O`, key, value) + return value as Value + } + +function Client() { + const [aRefCount, setARefCount] = useState(0) + const [bRefCount, setBRefCount] = useState(0) + const [a, setA] = useQueryState( + 'a', + parseAsJson(makeLoggingSpy('a')).withDefault(defaultValue) + ) + const [{ b }, setB] = useQueryStates({ + b: parseAsJson(makeLoggingSpy('b')).withDefault(defaultValue) + }) + + useEffect(() => { + setARefCount(old => old + 1) + }, [a]) + useEffect(() => { + setBRefCount(old => old + 1) + }, [b]) + + return ( + <> +
+ + + + + Refs seen: {aRefCount} + +
+
+ + + + + Refs seen: {bRefCount} + +
+
+ + Link to # + +
+ + ) +} From 850ca5e91dbfeb9ffca8576973471cfa3ad00eb6 Mon Sep 17 00:00:00 2001 From: Francois Best Date: Wed, 28 Aug 2024 16:42:43 +0200 Subject: [PATCH 2/4] fix: Ensure referential stablity --- packages/nuqs/src/update-queue.ts | 1 + packages/nuqs/src/useQueryState.ts | 13 ++++++- packages/nuqs/src/useQueryStates.ts | 53 +++++++++++++++++++---------- 3 files changed, 48 insertions(+), 19 deletions(-) diff --git a/packages/nuqs/src/update-queue.ts b/packages/nuqs/src/update-queue.ts index d38e035f5..03e6efb13 100644 --- a/packages/nuqs/src/update-queue.ts +++ b/packages/nuqs/src/update-queue.ts @@ -55,6 +55,7 @@ export function enqueueQueryStringUpdate( options.throttleMs ?? FLUSH_RATE_LIMIT_MS, Number.isFinite(queueOptions.throttleMs) ? queueOptions.throttleMs : 0 ) + return serializedOrNull } export function getQueuedValue(key: string) { diff --git a/packages/nuqs/src/useQueryState.ts b/packages/nuqs/src/useQueryState.ts index 78a023f2a..cb8d1e81d 100644 --- a/packages/nuqs/src/useQueryState.ts +++ b/packages/nuqs/src/useQueryState.ts @@ -225,10 +225,12 @@ export function useQueryState( const router = useRouter() // Not reactive, but available on the server and on page load const initialSearchParams = useSearchParams() + const valueRef = React.useRef(null) const [internalState, setInternalState] = React.useState(() => { const queueValue = getQueuedValue(key) const urlValue = initialSearchParams?.get(key) ?? null const value = queueValue ?? urlValue + valueRef.current = value return value === null ? null : safeParse(parse, value, key) }) const stateRef = React.useRef(internalState) @@ -246,9 +248,13 @@ export function useQueryState( return } const value = initialSearchParams.get(key) ?? null + if (value === valueRef.current) { + return + } const state = value === null ? null : safeParse(parse, value, key) debug('[nuqs `%s`] syncFromUseSearchParams %O', key, state) stateRef.current = state + valueRef.current = value setInternalState(state) }, [initialSearchParams?.get(key), key]) @@ -257,13 +263,18 @@ export function useQueryState( function updateInternalState(state: T | null) { debug('[nuqs `%s`] updateInternalState %O', key, state) stateRef.current = state + valueRef.current = state === null ? null : serialize(state) setInternalState(state) } function syncFromURL(search: URLSearchParams) { const value = search.get(key) ?? null + if (value === valueRef.current) { + return + } const state = value === null ? null : safeParse(parse, value, key) debug('[nuqs `%s`] syncFromURL %O', key, state) updateInternalState(state) + valueRef.current = value } debug('[nuqs `%s`] subscribing to sync', key) emitter.on(SYNC_EVENT_KEY, syncFromURL) @@ -290,7 +301,7 @@ export function useQueryState( } // Sync all hooks state (including this one) emitter.emit(key, newValue) - enqueueQueryStringUpdate(key, newValue, serialize, { + valueRef.current = enqueueQueryStringUpdate(key, newValue, serialize, { // Call-level options take precedence over hook declaration options. history: options.history ?? history, shallow: options.shallow ?? shallow, diff --git a/packages/nuqs/src/useQueryStates.ts b/packages/nuqs/src/useQueryStates.ts index 57953d6f8..8704331b0 100644 --- a/packages/nuqs/src/useQueryStates.ts +++ b/packages/nuqs/src/useQueryStates.ts @@ -73,9 +73,13 @@ export function useQueryStates( const router = useRouter() // Not reactive, but available on the server and on page load const initialSearchParams = useSearchParams() - const [internalState, setInternalState] = React.useState(() => - parseMap(keyMap, initialSearchParams ?? new URLSearchParams()) - ) + const queryRef = React.useRef>({}) + const [internalState, setInternalState] = React.useState(() => { + const source = initialSearchParams ?? new URLSearchParams() + queryRef.current = Object.fromEntries(source.entries()) + return parseMap(keyMap, source) + }) + const stateRef = React.useRef(internalState) debug( '[nuq+ `%s`] render - state: %O, iSP: %s', @@ -92,20 +96,21 @@ export function useQueryStates( setInternalState(state) } function syncFromURL(search: URLSearchParams) { - const state = parseMap(keyMap, search) + const state = parseMap(keyMap, search, queryRef.current, stateRef.current) debug('[nuq+ `%s`] syncFromURL %O', keys, state) updateInternalState(state) } const handlers = Object.keys(keyMap).reduce( (handlers, key) => { handlers[key as keyof V] = (value: any) => { - const { defaultValue } = keyMap[key]! + const { defaultValue, serialize = String } = keyMap[key]! // Note: cannot mutate in-place, the object ref must change // for the subsequent setState to pick it up. stateRef.current = { ...stateRef.current, [key as keyof V]: value ?? defaultValue ?? null } + queryRef.current[key] = value === null ? null : serialize(value) debug( '[nuq+ `%s`] Cross-hook key sync %s: %O (default: %O). Resolved: %O', keys, @@ -162,18 +167,24 @@ export function useQueryStates( value = null } emitter.emit(key, value) - enqueueQueryStringUpdate(key, value, parser.serialize ?? String, { - // Call-level options take precedence over individual parser options - // which take precedence over global options - history: callOptions.history ?? parser.history ?? history, - shallow: callOptions.shallow ?? parser.shallow ?? shallow, - scroll: callOptions.scroll ?? parser.scroll ?? scroll, - throttleMs: callOptions.throttleMs ?? parser.throttleMs ?? throttleMs, - startTransition: - callOptions.startTransition ?? - parser.startTransition ?? - startTransition - }) + queryRef.current[key] = enqueueQueryStringUpdate( + key, + value, + parser.serialize ?? String, + { + // Call-level options take precedence over individual parser options + // which take precedence over global options + history: callOptions.history ?? parser.history ?? history, + shallow: callOptions.shallow ?? parser.shallow ?? shallow, + scroll: callOptions.scroll ?? parser.scroll ?? scroll, + throttleMs: + callOptions.throttleMs ?? parser.throttleMs ?? throttleMs, + startTransition: + callOptions.startTransition ?? + parser.startTransition ?? + startTransition + } + ) } return scheduleFlushToURL(router) }, @@ -186,13 +197,19 @@ export function useQueryStates( function parseMap( keyMap: KeyMap, - searchParams: URLSearchParams | ReadonlyURLSearchParams + searchParams: URLSearchParams | ReadonlyURLSearchParams, + cachedQuery?: Record, + cachedState?: Values ) { return Object.keys(keyMap).reduce((obj, key) => { const { defaultValue, parse } = keyMap[key]! const urlQuery = searchParams?.get(key) ?? null const queueQuery = getQueuedValue(key) const query = queueQuery ?? urlQuery + if (cachedQuery && cachedState && cachedQuery[key] === query) { + obj[key as keyof KeyMap] = cachedState[key] ?? defaultValue ?? null + return obj + } const value = query === null ? null : safeParse(parse, query, key) obj[key as keyof KeyMap] = value ?? defaultValue ?? null return obj From 00beeeb8550c903635d0263406b2f67148b48f81 Mon Sep 17 00:00:00 2001 From: Francois Best Date: Wed, 28 Aug 2024 17:12:35 +0200 Subject: [PATCH 3/4] ref: Carry state & serialized query across hook sync So that internal refs can be synced together without calling the serializer on each reception site. --- packages/nuqs/src/sync.ts | 6 +++++- packages/nuqs/src/useQueryState.ts | 33 ++++++++++++++--------------- packages/nuqs/src/useQueryStates.ts | 22 +++++++++++-------- 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/packages/nuqs/src/sync.ts b/packages/nuqs/src/sync.ts index 685706c4d..fb0a476d7 100644 --- a/packages/nuqs/src/sync.ts +++ b/packages/nuqs/src/sync.ts @@ -12,11 +12,15 @@ export type QueryUpdateNotificationArgs = { search: URLSearchParams source: QueryUpdateSource } +export type CrossHookSyncPayload = { + state: any + query: string | null +} type EventMap = { [SYNC_EVENT_KEY]: URLSearchParams [NOTIFY_EVENT_KEY]: QueryUpdateNotificationArgs - [key: string]: any + [key: string]: CrossHookSyncPayload } export const emitter = Mitt() diff --git a/packages/nuqs/src/useQueryState.ts b/packages/nuqs/src/useQueryState.ts index cb8d1e81d..d13c5eef3 100644 --- a/packages/nuqs/src/useQueryState.ts +++ b/packages/nuqs/src/useQueryState.ts @@ -3,7 +3,7 @@ import React from 'react' import { debug } from './debug' import type { Options } from './defs' import type { Parser } from './parsers' -import { SYNC_EVENT_KEY, emitter } from './sync' +import { SYNC_EVENT_KEY, emitter, type CrossHookSyncPayload } from './sync' import { FLUSH_RATE_LIMIT_MS, enqueueQueryStringUpdate, @@ -225,12 +225,12 @@ export function useQueryState( const router = useRouter() // Not reactive, but available on the server and on page load const initialSearchParams = useSearchParams() - const valueRef = React.useRef(null) + const queryRef = React.useRef(null) const [internalState, setInternalState] = React.useState(() => { const queueValue = getQueuedValue(key) const urlValue = initialSearchParams?.get(key) ?? null const value = queueValue ?? urlValue - valueRef.current = value + queryRef.current = value return value === null ? null : safeParse(parse, value, key) }) const stateRef = React.useRef(internalState) @@ -247,34 +247,33 @@ export function useQueryState( if (window.next?.version !== '14.0.3') { return } - const value = initialSearchParams.get(key) ?? null - if (value === valueRef.current) { + const query = initialSearchParams.get(key) ?? null + if (query === queryRef.current) { return } - const state = value === null ? null : safeParse(parse, value, key) + const state = query === null ? null : safeParse(parse, query, key) debug('[nuqs `%s`] syncFromUseSearchParams %O', key, state) stateRef.current = state - valueRef.current = value + queryRef.current = query setInternalState(state) }, [initialSearchParams?.get(key), key]) // Sync all hooks together & with external URL changes React.useInsertionEffect(() => { - function updateInternalState(state: T | null) { + function updateInternalState({ state, query }: CrossHookSyncPayload) { debug('[nuqs `%s`] updateInternalState %O', key, state) stateRef.current = state - valueRef.current = state === null ? null : serialize(state) + queryRef.current = query setInternalState(state) } function syncFromURL(search: URLSearchParams) { - const value = search.get(key) ?? null - if (value === valueRef.current) { + const query = search.get(key) + if (query === queryRef.current) { return } - const state = value === null ? null : safeParse(parse, value, key) + const state = query === null ? null : safeParse(parse, query, key) debug('[nuqs `%s`] syncFromURL %O', key, state) - updateInternalState(state) - valueRef.current = value + updateInternalState({ state, query }) } debug('[nuqs `%s`] subscribing to sync', key) emitter.on(SYNC_EVENT_KEY, syncFromURL) @@ -299,9 +298,7 @@ export function useQueryState( ) { newValue = null } - // Sync all hooks state (including this one) - emitter.emit(key, newValue) - valueRef.current = enqueueQueryStringUpdate(key, newValue, serialize, { + queryRef.current = enqueueQueryStringUpdate(key, newValue, serialize, { // Call-level options take precedence over hook declaration options. history: options.history ?? history, shallow: options.shallow ?? shallow, @@ -309,6 +306,8 @@ export function useQueryState( throttleMs: options.throttleMs ?? throttleMs, startTransition: options.startTransition ?? startTransition }) + // Sync all hooks state (including this one) + emitter.emit(key, { state: newValue, query: queryRef.current }) return scheduleFlushToURL(router) }, [key, history, shallow, scroll, throttleMs, startTransition] diff --git a/packages/nuqs/src/useQueryStates.ts b/packages/nuqs/src/useQueryStates.ts index 8704331b0..26fefd221 100644 --- a/packages/nuqs/src/useQueryStates.ts +++ b/packages/nuqs/src/useQueryStates.ts @@ -7,7 +7,7 @@ import React from 'react' import { debug } from './debug' import type { Nullable, Options } from './defs' import type { Parser } from './parsers' -import { SYNC_EVENT_KEY, emitter } from './sync' +import { SYNC_EVENT_KEY, emitter, type CrossHookSyncPayload } from './sync' import { FLUSH_RATE_LIMIT_MS, enqueueQueryStringUpdate, @@ -102,20 +102,20 @@ export function useQueryStates( } const handlers = Object.keys(keyMap).reduce( (handlers, key) => { - handlers[key as keyof V] = (value: any) => { - const { defaultValue, serialize = String } = keyMap[key]! + handlers[key as keyof V] = ({ state, query }: CrossHookSyncPayload) => { + const { defaultValue } = keyMap[key]! // Note: cannot mutate in-place, the object ref must change // for the subsequent setState to pick it up. stateRef.current = { ...stateRef.current, - [key as keyof V]: value ?? defaultValue ?? null + [key as keyof V]: state ?? defaultValue ?? null } - queryRef.current[key] = value === null ? null : serialize(value) + queryRef.current[key] = query debug( '[nuq+ `%s`] Cross-hook key sync %s: %O (default: %O). Resolved: %O', keys, key, - value, + state, defaultValue, stateRef.current ) @@ -123,13 +123,13 @@ export function useQueryStates( } return handlers }, - {} as Record + {} as Record void> ) emitter.on(SYNC_EVENT_KEY, syncFromURL) for (const key of Object.keys(keyMap)) { debug('[nuq+ `%s`] Subscribing to sync for `%s`', keys, key) - emitter.on(key, handlers[key]) + emitter.on(key, handlers[key]!) } return () => { emitter.off(SYNC_EVENT_KEY, syncFromURL) @@ -166,7 +166,7 @@ export function useQueryStates( ) { value = null } - emitter.emit(key, value) + queryRef.current[key] = enqueueQueryStringUpdate( key, value, @@ -185,6 +185,10 @@ export function useQueryStates( startTransition } ) + emitter.emit(key, { + state: value, + query: queryRef.current[key] ?? null + }) } return scheduleFlushToURL(router) }, From 88d2c5e45ed8cde90db66792cf1060c704d90ed9 Mon Sep 17 00:00:00 2001 From: Francois Best Date: Fri, 30 Aug 2024 15:52:50 +0200 Subject: [PATCH 4/4] doc: Add caveat about multiple parsers on the same key --- .../docs/content/docs/troubleshooting.mdx | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/docs/content/docs/troubleshooting.mdx b/packages/docs/content/docs/troubleshooting.mdx index 01d6cfd2b..8f99a8c96 100644 --- a/packages/docs/content/docs/troubleshooting.mdx +++ b/packages/docs/content/docs/troubleshooting.mdx @@ -13,3 +13,32 @@ Because the Next.js **pages router** is not available in an SSR context, this hook will always return `null` (or the default value if supplied) on SSR/SSG. This limitation doesn't apply to the app router. + +## Caveats + +### Different parsers on the same key + +Hooks are synced together on a per-key bassis, so if you use different parsers +on the same key, the last state update will be propagated to all other hooks +using that key. It can lead to unexpected states like this: + +```ts +const [int] = useQueryState('foo', parseAsInteger) +const [float, setFloat] = useQueryState('foo', parseAsFloat) + +setFloat(1.234) + +// `int` is now 1.234, instead of 1 +``` + +We recommend you abstract a key/parser pair into a dedicated hook to avoid this, +and derive any desired state from the value: + +```ts +function useIntFloat() { + const [float, setFloat] = useQueryState('foo', parseAsFloat) + const int = Math.floor(float) + return [{int, float}, setFloat] as const +} +``` +