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

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

Closed
iAmGhost opened this issue Feb 17, 2022 · 7 comments · Fixed by #10427
Closed

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

iAmGhost opened this issue Feb 17, 2022 · 7 comments · Fixed by #10427
Labels

Comments

@iAmGhost
Copy link

iAmGhost commented Feb 17, 2022

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 iAmGhost added the bug label Feb 17, 2022
@bgrace
Copy link

bgrace commented Jun 30, 2022

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.

@ghost
Copy link

ghost commented Nov 15, 2022

+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).

@abenhamdine
Copy link

abenhamdine commented Mar 22, 2023

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 ! 🙏

@BrianHung
Copy link

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.

@abenhamdine
Copy link

abenhamdine commented Apr 3, 2023

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.

@brophdawg11
Copy link
Contributor

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 brophdawg11 self-assigned this Apr 28, 2023
@brophdawg11 brophdawg11 linked a pull request May 1, 2023 that will close this issue
@brophdawg11 brophdawg11 added the awaiting release This issue have been fixed and will be released soon label May 23, 2023
@brophdawg11 brophdawg11 removed their assignment May 23, 2023
@brophdawg11
Copy link
Contributor

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

@brophdawg11 brophdawg11 removed the awaiting release This issue have been fixed and will be released soon label Jun 6, 2023
brophdawg11 pushed a commit that referenced this issue Mar 27, 2024
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants