Skip to content

Commit

Permalink
fix: Shallow equality check on cache parsing input (#580)
Browse files Browse the repository at this point in the history
  • Loading branch information
franky47 authored Jun 26, 2024
1 parent bf8ab26 commit 4d4fc9e
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 16 deletions.
2 changes: 1 addition & 1 deletion packages/e2e/cypress/e2e/cache.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

describe('cache', () => {
it('works in app router', () => {
cy.visit('/app/cache?str=foo&num=42&bool=true')
cy.visit('/app/cache?str=foo&num=42&bool=true&multi=foo&multi=bar')
cy.get('#parse-str').should('have.text', 'foo')
cy.get('#parse-num').should('have.text', '42')
cy.get('#parse-bool').should('have.text', 'true')
Expand Down
39 changes: 37 additions & 2 deletions packages/nuqs/src/cache.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, it, vi } from 'vitest'
import { createSearchParamsCache } from './cache'
import { compareSearchParams, createSearchParamsCache } from './cache'
import { parseAsString } from './parsers'

// provide a simple mock for React cache
Expand All @@ -22,7 +22,7 @@ describe('cache', () => {
string: "I'm a string"
}

it('allows parsing same object multiple times in a request', () => {
it('allows parsing the same object multiple times in a request', () => {
const cache = createSearchParamsCache({
string: parseAsString
})
Expand All @@ -37,6 +37,15 @@ describe('cache', () => {
expect(cache.all()).toBe(all)
})

it('allows parsing the same content with different references', () => {
const cache = createSearchParamsCache({
string: parseAsString
})
const copy = { ...input }
expect(cache.parse(input).string).toBe(input.string)
expect(cache.parse(copy).string).toBe(input.string)
})

it('disallows parsing different objects in a request', () => {
const cache = createSearchParamsCache({
string: parseAsString
Expand All @@ -51,4 +60,30 @@ describe('cache', () => {
expect(cache.all()).toBe(all)
})
})

describe('compareSearchParams', () => {
it('works on empty search params', () => {
expect(compareSearchParams({}, {})).toBe(true)
})
it('rejects different lengths', () => {
expect(compareSearchParams({ a: 'a' }, { a: 'a', b: 'b' })).toBe(false)
})
it('rejects different values', () => {
expect(compareSearchParams({ x: 'a' }, { x: 'b' })).toBe(false)
})
it('does not care about order', () => {
expect(compareSearchParams({ x: 'a', y: 'b' }, { y: 'b', x: 'a' })).toBe(
true
)
})
it('supports array values (referentially stable)', () => {
const array = ['a', 'b']
expect(compareSearchParams({ x: array }, { x: array })).toBe(true)
})
it('does not do deep comparison', () => {
expect(compareSearchParams({ x: ['a', 'b'] }, { x: ['a', 'b'] })).toBe(
false
)
})
})
})
34 changes: 22 additions & 12 deletions packages/nuqs/src/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,28 +34,23 @@ export function createSearchParamsCache<
}))
function parse(searchParams: SearchParams) {
const c = getCache()

if (Object.isFrozen(c.searchParams)) {
// parse has already been called
if (searchParams === c[$input]) {
// but we're being called with the identical object again, so we can safely return the same cached result
// (an example of when this occurs would be if parse was called in generateMetadata as well as the page itself).
// note that this simply checks for referential equality and will still fail if a different object with the
// same contents is passed. fortunately next.js uses the same object for search params in the same request.
// Parse has already been called...
if (c[$input] && compareSearchParams(searchParams, c[$input])) {
// ...but we're being called with the same contents again,
// so we can safely return the same cached result (an example of when
// this occurs would be if parse was called in generateMetadata as well
// as the page itself).
return all()
}

// different input in the same request - fail
// Different inputs in the same request - fail
throw new Error(error(501))
}

for (const key in parsers) {
const parser = parsers[key]!
c.searchParams[key] = parser.parseServerSide(searchParams[key])
}

c[$input] = searchParams

return Object.freeze(c.searchParams) as Readonly<ParsedSearchParams>
}
function all() {
Expand All @@ -80,3 +75,18 @@ export function createSearchParamsCache<
}
return { parse, get, all }
}

export function compareSearchParams(a: SearchParams, b: SearchParams) {
if (a === b) {
return true
}
if (Object.keys(a).length !== Object.keys(b).length) {
return false
}
for (const key in a) {
if (a[key] !== b[key]) {
return false
}
}
return true
}
2 changes: 1 addition & 1 deletion packages/nuqs/src/index.server.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export * from './cache'
export { createSearchParamsCache, type SearchParams } from './cache'
export * from './parsers'
export { createSerializer } from './serializer'

0 comments on commit 4d4fc9e

Please sign in to comment.