react-router icon indicating copy to clipboard operation
react-router copied to clipboard

[v6][Bug]: navigate() with non-serializable state should throw error

Open iAmGhost opened this issue 3 years ago • 2 comments

What version of React Router are you using?

6.2.1

Steps to Reproduce

Pass non serializable state to navigate()

Code for reproduction

https://codesandbox.io/s/react-router-navigate-bug-d9mlsn

import React from "react";
import { BrowserRouter } from "react-router-dom";
import { Routes, Route, useNavigate, useLocation } from "react-router";

import "./styles.css";

export default function App() {
  return (
    <div className="App">
      <BrowserRouter>
        <Routes>
          <Route path="/" element={<MainPage />} />
        </Routes>
      </BrowserRouter>
    </div>
  );
}

function MainPage() {
  const location = useLocation();

  const navigate = useNavigate();

  return (
    <>
      <p>{location.state?.title}</p>
      <p>
        <button
          onClick={() => {
            navigate("/", {
              state: {
                title: "Hello!"
              }
            });
          }}
        >
          Serializable
        </button>
      </p>
      <p>
        <button
          onClick={() => {
            navigate("/", {
              state: {
                title: <>Hello!</>
              }
            });
          }}
        >
          Non-Serializable
        </button>
      </p>
    </>
  );
}

Expected Behavior

Fail with some error since state must be serializable.

Actual Behavior

It changes browser's location instead of manipulating react-router's state without error. which makes devloper harder to find about their mistake because of wierd behavior.

iAmGhost avatar Feb 17 '22 04:02 iAmGhost

Seconding this. I tried to add a function as a member of the state object. While it's quite clear from the documents that "Location state values will get serialized, so something like new Date() will be turned into a string," this seems like something that the library should warn about and reject. The actual behavior—the router reloads the URL from the upstream server—sent me on a wild goose chase for a bug in my routing logic.

bgrace avatar Jun 30 '22 23:06 bgrace

+1 on this. Took me a lot to debug why my page gets reloaded when I pass the state to Link component (in my case I was passing down moment object, had to transform it to a string).

ghost avatar Nov 15 '22 17:11 ghost

I think I have the same problem with the Link component, with which I try to pass a component in the state. It's worth to note that it was working fine with RR5 and history 4.

it tooks me 3 hours to understand where the bug originate please maintainers add a warning/error message in this case ! 🙏

abenhamdine avatar Mar 22 '23 09:03 abenhamdine

Also think a development only error message or warning would be great, since it requires a bit of digging to find that you cant pass non-serializable state into navigate.

BrianHung avatar Apr 02 '23 20:04 BrianHung

IMO, a correct documentation should also link to the specification of what is "serializable". Following Web Api Browser specification , the serialization algorithm used is described there : https://html.spec.whatwg.org/multipage/structured-data.html#structuredserializeinternal

Mainly, Functions and Symbols cannot be serialized.

abenhamdine avatar Apr 03 '23 07:04 abenhamdine

Looks like we can detect this and re-throw via error instanceof DOMException && error.name === "DataCloneError"

https://html.spec.whatwg.org/multipage/nav-history-apis.html#shared-history-push/replace-state-steps https://html.spec.whatwg.org/multipage/structured-data.html#structuredserializeinternal

brophdawg11 avatar Apr 28 '23 21:04 brophdawg11

This is fixed by #10427 and will be available once the next release goes out (likely 6.12.0)

brophdawg11 avatar May 23 '23 14:05 brophdawg11