Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Equality check for non-primitives in clearOnDefault #504

Merged
merged 2 commits into from
Mar 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading