fix: don't fail parse if called again with identical input (#557)

* fix: don't fail parse if called again with identical input

* test: add e2e test

* style: run prettier

* fix: use a symbol to avoid conflict

* style: run prettier

* fix: freeze searchParams

Author: rikbrown

packages/e2e/cypress/e2e/cache.cy.js

@@ -18,5 +18,6 @@ describe('cache', () => {
     cy.get('#get-bool').should('have.text', 'true')
     cy.get('#get-def').should('have.text', 'default')
     cy.get('#get-nope').should('have.text', 'null')
+    cy.title().should('eq', 'metadata-title-str:foo')
   })
 })

packages/e2e/src/app/app/cache/page.tsx

@@ -4,11 +4,11 @@ import { Get } from './get'
 import { cache } from './searchParams'
 import { Set } from './set'
 
-export default function Page({
-  searchParams
-}: {
+type Props = {
   searchParams: Record<string, string | string[] | undefined>
-}) {
+}
+
+export default function Page({ searchParams }: Props) {
   const { str, bool, num, def, nope } = cache.parse(searchParams)
   return (
     <>
@@ -29,3 +29,11 @@ export default function Page({
     </>
   )
 }
+
+export function generateMetadata({ searchParams }: Props) {
+  // parse here too to ensure we can idempotently parse the same search params as the page in the same request
+  const { str } = cache.parse(searchParams)
+  return {
+    title: `metadata-title-str:${str}`
+  }
+}

packages/nuqs/src/cache.test.ts

@@ -0,0 +1,54 @@
+import { describe, expect, it, vi } from 'vitest'
+import { createSearchParamsCache } from './cache'
+import { parseAsString } from './parsers'
+
+// provide a simple mock for React cache
+vi.mock('react', () => {
+  return {
+    cache<T, CachedFunction extends () => T>(fn: CachedFunction) {
+      let cache: T | undefined = undefined
+      function cachedFn() {
+        cache ??= fn()
+        return cache
+      }
+      return cachedFn
+    }
+  }
+})
+
+describe('cache', () => {
+  describe('createSearchParamsCache', () => {
+    const input = {
+      string: "I'm a string"
+    }
+
+    it('allows parsing same object multiple times in a request', () => {
+      const cache = createSearchParamsCache({
+        string: parseAsString
+      })
+
+      // parse the input and perform some sanity checks
+      expect(cache.parse(input).string).toBe(input.string)
+      const all = cache.all()
+      expect(all.string).toBe(input.string)
+
+      // second call should be successful and return the same result
+      expect(cache.parse(input).string).toBe(input.string)
+      expect(cache.all()).toBe(all)
+    })
+
+    it('disallows parsing different objects in a request', () => {
+      const cache = createSearchParamsCache({
+        string: parseAsString
+      })
+      expect(cache.parse(input).string).toBe(input.string)
+      const all = cache.all()
+
+      // second call with different object should fail
+      expect(() => cache.parse({ string: 'I am a different string' })).toThrow()
+
+      // cache still works though
+      expect(cache.all()).toBe(all)
+    })
+  })
+})

packages/nuqs/src/cache.ts

@@ -4,6 +4,8 @@ import type { ParserBuilder } from './parsers'
 
 export type SearchParams = Record<string, string | string[] | undefined>
 
+const $input: unique symbol = Symbol('Input')
+
 type ExtractParserType<Parser> =
   Parser extends ParserBuilder<any>
     ? ReturnType<Parser['parseServerSide']>
@@ -16,33 +18,56 @@ export function createSearchParamsCache<
   type ParsedSearchParams = {
     [K in Keys]: ExtractParserType<Parsers[K]>
   }
+
+  type Cache = {
+    searchParams: Partial<ParsedSearchParams>
+    [$input]?: SearchParams
+  }
+
   // Why not use a good old object here ?
   // React's `cache` is bound to the render lifecycle of a page,
   // whereas a simple object would be bound to the lifecycle of the process,
   // which may be reused between requests in a serverless environment
   // (warm lambdas on Vercel or AWS).
-  const getCache = cache<() => Partial<ParsedSearchParams>>(() => ({}))
+  const getCache = cache<() => Cache>(() => ({
+    searchParams: {}
+  }))
   function parse(searchParams: SearchParams) {
     const c = getCache()
-    if (Object.isFrozen(c)) {
+
+    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.
+        return all()
+      }
+
+      // different input in the same request - fail
       throw new Error(error(501))
     }
+
     for (const key in parsers) {
       const parser = parsers[key]!
-      c[key] = parser.parseServerSide(searchParams[key])
+      c.searchParams[key] = parser.parseServerSide(searchParams[key])
     }
-    return Object.freeze(c) as Readonly<ParsedSearchParams>
+
+    c[$input] = searchParams
+
+    return Object.freeze(c.searchParams) as Readonly<ParsedSearchParams>
   }
   function all() {
-    const c = getCache()
-    if (Object.keys(c).length === 0) {
+    const { searchParams } = getCache()
+    if (Object.keys(searchParams).length === 0) {
       throw new Error(error(500))
     }
-    return c as Readonly<ParsedSearchParams>
+    return searchParams as Readonly<ParsedSearchParams>
   }
   function get<Key extends Keys>(key: Key): ParsedSearchParams[Key] {
-    const c = getCache()
-    const entry = c[key]
+    const { searchParams } = getCache()
+    const entry = searchParams[key]
     if (typeof entry === 'undefined') {
       throw new Error(
         error(500) +