Skip to content

Commit

Permalink
fix: State updater function should be based on state
Browse files Browse the repository at this point in the history 
Not the current URLSearchParams which may be stale if
two updates with computed state occur before the next flush.

This uses the Lastest Ref pattern:
https://epicreact.dev/the-latest-ref-pattern-in-react/

Add debug printout prefixes to make sense of it all.
  • Loading branch information
@franky47
franky47 committed Sep 15, 2023
1 parent 1e6815b commit f788381
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 65 deletions.
9 changes: 6 additions & 3 deletions src/lib/sync.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,16 @@ if (!patched && typeof window === 'object') {
// Null URL is only used for state changes,
// we're not interested in reacting to those.
__DEBUG__ &&
console.debug(`history.${method}(null) (${title}) %O`, state)
console.debug(`[nuqs] history.${method}(null) (${title}) %O`, state)
return original(state, title, url)
}
const source = title === NOSYNC_MARKER ? 'internal' : 'external'
const search = new URL(url, location.origin).searchParams
__DEBUG__ &&
console.debug(`history.${method}(${url}) (${source}) %O`, state)
console.debug(`[nuqs] history.${method}(${url}) (${source}) %O`, state)
// If someone else than our hooks have updated the URL,
// send out a signal for them to sync their internal state.
if (source === 'external') {
__DEBUG__ && console.debug(`Triggering sync with ${search.toString()}`)
// Here we're delaying application to next tick to avoid:
// `Warning: useInsertionEffect must not schedule updates.`
//
Expand All @@ -62,6 +61,10 @@ if (!patched && typeof window === 'object') {
// parsed query string to the hooks so they don't need
// to rely on the URL being up to date.
setTimeout(() => {
__DEBUG__ &&
console.debug(
`[nuqs] External history.${method} call: triggering sync with ${search.toString()}`
)
emitter.emit(SYNC_EVENT_KEY, search)
emitter.emit(NOTIFY_EVENT_KEY, { search, source })
}, 0)
Expand Down
9 changes: 5 additions & 4 deletions src/lib/update-queue.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export function enqueueQueryStringUpdate<Value>(
value: value === null ? null : serialize(value),
options
}
__DEBUG__ && console.debug('Pushing to queue %O', queueItem)
__DEBUG__ && console.debug('[nuqs queue] Pushing to queue %O', queueItem)
updateQueue.push(queueItem)
}

Expand All @@ -49,7 +49,8 @@ export function flushToURL(router: Router) {
0,
Math.min(FLUSH_RATE_LIMIT_MS, FLUSH_RATE_LIMIT_MS - timeSinceLastFlush)
)
__DEBUG__ && console.debug('Scheduling flush in %f ms', flushInMs)
__DEBUG__ &&
console.debug('[nuqs queue] Scheduling flush in %f ms', flushInMs)
setTimeout(() => {
lastFlushTimestamp = performance.now()
const search = flushUpdateQueue(router)
Expand All @@ -73,7 +74,7 @@ function flushUpdateQueue(router: Router) {
// Work on a copy and clear the queue immediately
const items = updateQueue.slice()
updateQueue = []
__DEBUG__ && console.debug('Flushing queue %O', items)
__DEBUG__ && console.debug('[nuqs queue] Flushing queue %O', items)

const options: Required<Options> = {
history: 'replace',
Expand Down Expand Up @@ -105,7 +106,7 @@ function flushUpdateQueue(router: Router) {
// otherwise using a relative URL works just fine.
// todo: Does it when using the router with `shallow: false` on dynamic paths?
const url = query ? `?${query}${hash}` : `${path}${hash}`
__DEBUG__ && console.debug('Updating url %s', url)
__DEBUG__ && console.debug('[nuqs queue] Updating url: %s', url)
try {
if (options.shallow) {
const updateUrl =
Expand Down
64 changes: 35 additions & 29 deletions src/lib/useQueryState.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ export function useQueryState<T = string>(
const initialSearchParams = useSearchParams()
__DEBUG__ &&
console.debug(
`initialSearchParams: ${
`[nuqs \`${key}\`] initialSearchParams: ${
initialSearchParams === null ? 'null' : initialSearchParams.toString()
}`
)
Expand All @@ -224,48 +224,42 @@ export function useQueryState<T = string>(
new URLSearchParams(window.location.search).get(key) ?? null
return value === null ? null : parse(value)
})
__DEBUG__ && console.debug(`render \`${key}\`: ${internalState}`)
const stateRef = React.useRef(internalState)
__DEBUG__ && console.debug(`[nuqs \`${key}\`] render: ${internalState}`)

// Sync all hooks together & with external URL changes
React.useInsertionEffect(() => {
function updateInternalState(state: T | null) {
__DEBUG__ &&
console.debug(`[nuqs \`${key}\`] updateInternalState %O`, state)
stateRef.current = state
setInternalState(state)
}
function syncFromURL(search: URLSearchParams) {
const value = search.get(key) ?? null
const v = value === null ? null : parse(value)
__DEBUG__ && console.debug(`sync \`${key}\`: ${v}`)
setInternalState(v)
const state = value === null ? null : parse(value)
__DEBUG__ && console.debug(`[nuqs \`${key}\`] syncFromURL: %O`, state)
updateInternalState(state)
}
__DEBUG__ && console.debug(`Subscribing to sync for \`${key}\``)
emitter.on(key, setInternalState)
__DEBUG__ && console.debug(`[nuqs \`${key}\`] Subscribing to sync`)

emitter.on(SYNC_EVENT_KEY, syncFromURL)
emitter.on(key, updateInternalState)
return () => {
__DEBUG__ && console.debug(`Unsubscribing from sync for \`${key}\``)
emitter.off(key, setInternalState)
__DEBUG__ && console.debug(`[nuqs \`${key}\`] Unsubscribing from sync`)
emitter.off(SYNC_EVENT_KEY, syncFromURL)
emitter.off(key, updateInternalState)
}
}, [key])

const update = React.useCallback(
(stateUpdater: React.SetStateAction<T | null>, options: Options = {}) => {
const isUpdaterFunction = (
input: any
): input is (prevState: T | null) => T | null => {
return typeof input === 'function'
}

let newValue: T | null = null
if (isUpdaterFunction(stateUpdater)) {
// Resolve the new value based on old value & updater
const search = new URLSearchParams(window.location.search)
const serialized = search.get(key) ?? null
const oldValue =
serialized === null
? defaultValue ?? null
: parse(serialized) ?? defaultValue ?? null
newValue = stateUpdater(oldValue)
} else {
newValue = stateUpdater
}
const newValue: T | null = isUpdaterFunction(stateUpdater)
? stateUpdater(stateRef.current ?? defaultValue ?? null)
: stateUpdater

__DEBUG__ &&
console.debug(`[nuqs \`${key}\`] Emitting and queueing: %O`, newValue)
// Sync all hooks state (including this one)
emitter.emit(key, newValue)
enqueueQueryStringUpdate(key, newValue, serialize, {
Expand All @@ -276,7 +270,19 @@ export function useQueryState<T = string>(
})
return flushToURL(router)
},
[key, history, shallow, scroll]
[
key,
history,
shallow,
scroll
// internalState, defaultValue
]
)
return [internalState ?? defaultValue ?? null, update]
}

function isUpdaterFunction<T>(
stateUpdater: React.SetStateAction<T>
): stateUpdater is (prevState: T) => T {
return typeof stateUpdater === 'function'
}
88 changes: 59 additions & 29 deletions src/lib/useQueryStates.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export function useQueryStates<KeyMap extends UseQueryStatesKeysMap>(
const initialSearchParams = useSearchParams()
__DEBUG__ &&
console.debug(
`initialSearchParams: ${
`[nuq+ \`${Object.keys(keyMap).join(',')}\`] initialSearchParams: ${
initialSearchParams === null ? 'null' : initialSearchParams.toString()
}`
)
Expand All @@ -75,71 +75,101 @@ export function useQueryStates<KeyMap extends UseQueryStatesKeysMap>(
// Components mounted after page load must use the current URL value
return parseMap(keyMap, new URLSearchParams(window.location.search))
})
const stateRef = React.useRef(internalState)
__DEBUG__ &&
console.debug(
`render \`${Object.keys(keyMap).join(',')}\`: %O`,
`[nuq+ \`${Object.keys(keyMap).join(',')}\`] render : %O`,
internalState
)

// Sync all hooks together & with external URL changes
React.useInsertionEffect(() => {
function updateInternalState(state: V) {
__DEBUG__ &&
console.debug(
`[nuq+ \`${Object.keys(keyMap).join(',')}\`] updateInternalState %O`,
state
)
stateRef.current = state
setInternalState(state)
}
function syncFromURL(search: URLSearchParams) {
const state = parseMap(keyMap, search)
__DEBUG__ &&
console.debug(`sync \`${Object.keys(keyMap).join(',')}\`: %O`, state)
setInternalState(state)
console.debug(
`[nuq+ \`${Object.keys(keyMap).join(',')}\`] syncFromURL: %O`,
state
)
updateInternalState(state)
}
const handlers = Object.keys(keyMap).reduce((handlers, key) => {
handlers[key as keyof V] = (value: any) => {
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
}
__DEBUG__ &&
console.debug(
`Cross-hook syncing \`${key}\`: %O (default: %O)`,
`[nuq+ \`${Object.keys(keyMap).join(
','
)}\`] Cross-hook key sync \`${key}\`: %O (default: %O). Resolved state: %O`,
value,
defaultValue
defaultValue,
stateRef.current
)
setInternalState(state => ({
...state,
[key]: value ?? defaultValue ?? null
}))
updateInternalState(stateRef.current)
}
return handlers
}, {} as Record<keyof V, any>)

emitter.on(SYNC_EVENT_KEY, syncFromURL)
for (const key of Object.keys(keyMap)) {
__DEBUG__ && console.debug(`Subscribing to sync for \`${key}\``)
__DEBUG__ &&
console.debug(
`[nuq+ \`${Object.keys(keyMap).join(
','
)}\`] Subscribing to sync for \`${key}\``
)
emitter.on(key, handlers[key])
}
emitter.on(SYNC_EVENT_KEY, syncFromURL)
return () => {
emitter.off(SYNC_EVENT_KEY, syncFromURL)
for (const key of Object.keys(keyMap)) {
__DEBUG__ && console.debug(`Unsubscribing from sync for \`${key}\``)
__DEBUG__ &&
console.debug(
`[nuq+ \`${Object.keys(keyMap).join(
','
)}\`] Unsubscribing to sync for \`${key}\``
)
emitter.off(key, handlers[key])
}
emitter.off(SYNC_EVENT_KEY, syncFromURL)
}
}, [keyMap])

const update = React.useCallback<SetValues<KeyMap>>(
(stateUpdater, options = {}) => {
const isUpdaterFunction = (input: any): input is UpdaterFn<KeyMap> => {
return typeof input === 'function'
}

// Resolve the new values based on old values & updater
const search = new URLSearchParams(window.location.search)
let newState: Partial<Nullable<KeyMap>> = {}
if (isUpdaterFunction(stateUpdater)) {
// todo: Should oldState contain null/undefined queries if not set?
const oldState = parseMap(keyMap, search)
newState = stateUpdater(oldState)
} else {
newState = stateUpdater
}
__DEBUG__ && console.debug(`Setting state: %O`, newState)
const newState: Partial<Nullable<KeyMap>> =
typeof stateUpdater === 'function'
? stateUpdater(stateRef.current)
: stateUpdater
__DEBUG__ &&
console.debug(
`[nuq+ \`${Object.keys(keyMap).join(',')}\`] setState: %O`,
newState
)
for (const [key, value] of Object.entries(newState)) {
const { serialize } = keyMap[key]
emitter.emit(key, value)
__DEBUG__ &&
console.debug(
`[nuq+ \`${Object.keys(keyMap).join(
','
)}\`] Emitting and queueing: \`${key}\`: %O`,
value
)
enqueueQueryStringUpdate(key, value, serialize ?? String, {
// Call-level options take precedence over hook declaration options.
history: options.history ?? history,
Expand Down

0 comments on commit f788381

Please sign in to comment.