Handle special props ref and key in path and search params (#5537)

Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>

packages/router/src/__tests__/router.test.tsx

@@ -1489,3 +1489,23 @@ test('params should be updated if navigated to different route with same page',
     expect(screen.queryByText('hookparams 99')).toBeInTheDocument()
   })
 })
+
+test('should handle ref and key as search params', async () => {
+  const ParamsPage = () => {
+    const { ref, key } = useParams()
+    return <p>{JSON.stringify({ ref, key })}</p>
+  }
+
+  const TestRouter = () => (
+    <Router>
+      <Route path="/params" page={ParamsPage} name="params" />
+    </Router>
+  )
+
+  const screen = render(<TestRouter />)
+  act(() => navigate('/params?ref=1&key=2'))
+
+  await waitFor(() => {
+    expect(screen.queryByText(`{"ref":"1","key":"2"}`)).toBeInTheDocument()
+  })
+})

packages/router/src/__tests__/util.test.ts

@@ -283,6 +283,39 @@ describe('validatePath', () => {
   ])('validates correct path "%s"', (path) => {
     expect(validatePath.bind(null, path)).not.toThrow()
   })
+
+  it.each([
+    '/path/{ref}',
+    '/path/{ref}/bazinga',
+    '/path/{ref:Int}',
+    '/path/{ref:Int}/bazinga',
+    '/path/{key}',
+    '/path/{key}/bazinga',
+    '/path/{key:Int}',
+  ])('rejects paths with ref or key as path parameters: "%s"', (path) => {
+    expect(validatePath.bind(null, path)).toThrowError(
+      [
+        `Route contains ref or key as a path parameter: "${path}"`,
+        "`ref` and `key` shouldn't be used as path parameters because they're special React props.",
+        'You can fix this by renaming the path parameter.',
+      ].join('\n')
+    )
+  })
+
+  it.each([
+    '/path/{reff}',
+    '/path/{reff:Int}',
+    '/path/{reff}/bazinga',
+    '/path/{keys}',
+    '/path/{keys:Int}',
+    '/path/key',
+    '/path/key/bazinga',
+  ])(
+    `doesn't reject paths with variations on ref or key as path parameters: "%s"`,
+    (path) => {
+      expect(validatePath.bind(null, path)).not.toThrowError()
+    }
+  )
 })
 
 describe('parseSearch', () => {

packages/router/src/router.tsx

@@ -97,7 +97,7 @@ const InternalRoute = ({
   )
 
   const searchParams = parseSearch(location.search)
-  const allParams = { ...searchParams, ...pathParams }
+  const allParams: Record<string, string> = { ...searchParams, ...pathParams }
 
   if (redirect) {
     const newPath = replaceParams(redirect, allParams)
@@ -113,6 +113,14 @@ const InternalRoute = ({
 
   const Page = activePageContext.loadingState[path]?.page || (() => null)
 
+  // There are two special props in React: `ref` and `key`. (See https://reactjs.org/warnings/special-props.html.)
+  // It's very possible that the URL has `ref` as a search param (e.g. https://redwoodjs.com/?ref=producthunt).
+  // Since we pass URL params to the page, we have to be careful not to pass `ref` or `key`, otherwise the page will break.
+  // (The page won't actually break if `key` is passed, but it feels unclean.)
+  // If users want to access them, they can use `useParams`.
+  delete allParams['ref']
+  delete allParams['key']
+
   // Level 3/3 (InternalRoute)
   return <Page {...allParams} />
 }

packages/router/src/util.ts

@@ -197,6 +197,16 @@ export const validatePath = (path: string) => {
     throw new Error(`Route path contains spaces: "${path}"`)
   }
 
+  if (/{(?:ref|key)(?::|})/.test(path)) {
+    throw new Error(
+      [
+        `Route contains ref or key as a path parameter: "${path}"`,
+        "`ref` and `key` shouldn't be used as path parameters because they're special React props.",
+        'You can fix this by renaming the path parameter.',
+      ].join('\n')
+    )
+  }
+
   // Check for duplicate named params.
   const matches = path.matchAll(/\{([^}]+)\}/g)
   const memo: Record<string, boolean> = {}