Skip to content

Commit

Permalink
fix: Equality check for non-primitives in clearOnDefault (#504)
Browse files Browse the repository at this point in the history
* fix: Equality check for non-primitives in clearOnDefault

* doc: Add JSDoc for the Parser type
  • Loading branch information
franky47 authored Mar 2, 2024
1 parent dea5afa commit e8c9720
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 8 deletions.
4 changes: 3 additions & 1 deletion packages/e2e/cypress/e2e/clearOnDefault.cy.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
/// <reference types="cypress" />

it('Clears the URL when setting the default value when `clearOnDefault` is used', () => {
cy.visit('/app/clearOnDefault?a=a&b=b')
cy.visit(
'/app/clearOnDefault?a=a&b=b&array=1,2,3&json-ref={"egg":"spam"}&json-new={"egg":"spam"}'
)
cy.contains('#hydration-marker', 'hydrated').should('be.hidden')
cy.get('button').click()
cy.location('search').should('eq', '?a=')
Expand Down
26 changes: 25 additions & 1 deletion packages/e2e/src/app/app/clearOnDefault/page.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
'use client'

import { useQueryState } from 'nuqs'
import {
parseAsArrayOf,
parseAsInteger,
parseAsJson,
useQueryState
} from 'nuqs'
import { Suspense } from 'react'

export default function Page() {
Expand All @@ -11,18 +16,37 @@ export default function Page() {
)
}

const defaultJSON = { foo: 'bar' }

function Client() {
const [, setA] = useQueryState('a')
const [, setB] = useQueryState('b', {
defaultValue: '',
clearOnDefault: true
})
const [, setArray] = useQueryState(
'array',
parseAsArrayOf(parseAsInteger)
.withDefault([])
.withOptions({ clearOnDefault: true })
)
const [, setJsonRef] = useQueryState(
'json-ref',
parseAsJson().withDefault(defaultJSON).withOptions({ clearOnDefault: true })
)
const [, setJsonNew] = useQueryState(
'json-new',
parseAsJson().withDefault(defaultJSON).withOptions({ clearOnDefault: true })
)
return (
<>
<button
onClick={() => {
setA('')
setB('')
setArray([])
setJsonRef(defaultJSON)
setJsonNew({ ...defaultJSON })
}}
>
Clear
Expand Down
2 changes: 1 addition & 1 deletion packages/nuqs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
{
"name": "Client (ESM)",
"path": "dist/index.js",
"limit": "4 kB",
"limit": "5 kB",
"ignore": [
"react"
]
Expand Down
11 changes: 11 additions & 0 deletions packages/nuqs/src/parsers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,14 @@ describe('parsers', () => {
expect(p.parseServerSide(undefined)).toBe('bar')
})
})

describe('parsers/equality', () => {
test('parseAsArrayOf', () => {
const eq = parseAsArrayOf(parseAsString).eq!
expect(eq([], [])).toBe(true)
expect(eq(['foo'], ['foo'])).toBe(true)
expect(eq(['foo', 'bar'], ['foo', 'bar'])).toBe(true)
expect(eq([], ['foo'])).toBe(false)
expect(eq(['foo'], ['bar'])).toBe(false)
})
})
46 changes: 43 additions & 3 deletions packages/nuqs/src/parsers.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,32 @@
import type { Options } from './defs'
import { safeParse } from './utils'

type Require<T, Keys extends keyof T> = Pick<Required<T>, Keys> & Omit<T, Keys>

export type Parser<T> = {
/**
* Convert a query string value into a state value.
*
* If the string value does not represent a valid state value,
* the parser should return `null`. Throwing an error is also supported.
*/
parse: (value: string) => T | null

/**
* Render the state value into a query string value.
*/
serialize?: (value: T) => string

/**
* Check if two state values are equal.
*
* This is used when using the `clearOnDefault` value, to compare the default
* value with the set value.
*
* It makes sense to provide this function when the state value is an object
* or an array, as the default referential equality check will not work.
*/
eq?: (a: T, b: T) => boolean
}

export type ParserBuilder<T> = Required<Parser<T>> &
Expand Down Expand Up @@ -70,7 +93,9 @@ export type ParserBuilder<T> = Required<Parser<T>> &
* Wrap a set of parse/serialize functions into a builder pattern parser
* you can pass to one of the hooks, making its default value type safe.
*/
export function createParser<T>(parser: Required<Parser<T>>): ParserBuilder<T> {
export function createParser<T>(
parser: Require<Parser<T>, 'parse' | 'serialize'>
): ParserBuilder<T> {
function parseServerSideNullable(value: string | string[] | undefined) {
if (typeof value === 'undefined') {
return null
Expand All @@ -91,6 +116,7 @@ export function createParser<T>(parser: Required<Parser<T>>): ParserBuilder<T> {
}

return {
eq: (a, b) => a === b,
...parser,
parseServerSide: parseServerSideNullable,
withDefault(defaultValue) {
Expand Down Expand Up @@ -318,7 +344,11 @@ export function parseAsJson<T>(parser?: (value: unknown) => T) {
return null
}
},
serialize: value => JSON.stringify(value)
serialize: value => JSON.stringify(value),
eq(a, b) {
// Check for referential equality first
return a === b || JSON.stringify(a) === JSON.stringify(b)
}
})
}

Expand All @@ -333,6 +363,7 @@ export function parseAsArrayOf<ItemType>(
itemParser: Parser<ItemType>,
separator = ','
) {
const itemEq = itemParser.eq ?? ((a: ItemType, b: ItemType) => a === b)
const encodedSeparator = encodeURIComponent(separator)
// todo: Handle default item values and make return type non-nullable
return createParser({
Expand Down Expand Up @@ -361,6 +392,15 @@ export function parseAsArrayOf<ItemType>(
: String(value)
return str.replaceAll(separator, encodedSeparator)
})
.join(separator)
.join(separator),
eq(a, b) {
if (a === b) {
return true // Referentially stable
}
if (a.length !== b.length) {
return false
}
return a.every((value, index) => itemEq(value, b[index]!))
}
})
}
6 changes: 5 additions & 1 deletion packages/nuqs/src/useQueryState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ export function useQueryState<T = string>(
throttleMs = FLUSH_RATE_LIMIT_MS,
parse = x => x as unknown as T,
serialize = String,
eq = (a, b) => a === b,
defaultValue = undefined,
clearOnDefault = false,
startTransition
Expand All @@ -216,6 +217,7 @@ export function useQueryState<T = string>(
throttleMs: FLUSH_RATE_LIMIT_MS,
parse: x => x as unknown as T,
serialize: String,
eq: (a, b) => a === b,
clearOnDefault: false,
defaultValue: undefined
}
Expand Down Expand Up @@ -285,7 +287,9 @@ export function useQueryState<T = string>(
: stateUpdater
if (
(options.clearOnDefault || clearOnDefault) &&
newValue === defaultValue
newValue !== null &&
defaultValue !== undefined &&
eq(newValue, defaultValue)
) {
newValue = null
}
Expand Down
4 changes: 3 additions & 1 deletion packages/nuqs/src/useQueryStates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ export function useQueryStates<KeyMap extends UseQueryStatesKeysMap>(
}
if (
(options.clearOnDefault || clearOnDefault) &&
value === config.defaultValue
value !== null &&
config.defaultValue !== undefined &&
(config.eq ?? ((a, b) => a === b))(value, config.defaultValue)
) {
value = null
}
Expand Down

0 comments on commit e8c9720

Please sign in to comment.