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

Passing state option in navigate doesn't seem to work #96

Closed
websymphony opened this issue Jul 6, 2018 · 40 comments
Closed

Passing state option in navigate doesn't seem to work #96

websymphony opened this issue Jul 6, 2018 · 40 comments
Labels
needs docs Common questions that could be a guide in the docs, or missing API documentation

Comments

@websymphony
Copy link

When state option is provided to navigate, it is not available in the props.location.state object for the matched components.

Example sandbox: https://codesandbox.io/s/l27754822m

Steps to reproduce:

  • Click on 'Invoices' link.
  • Add some ID in the input field.
  • Click on 'create' button.
  • Watch console which is logging props.location.state for Invoice and Invoices components.

I have taken the example from documentation and updated the Invoices component to have state option { invoiceID: provided_id } for the navigate function.
props.location.state seems to only have key and no other information for the matched Invoice and Invoices components.

Is it a bug or am I not using the option correctly? 😕

@milamer
Copy link

milamer commented Jul 12, 2018

I think the documentation is wrong.
You can add a state by providing navigate with an options object with a state property, the data under the state property will then be merged with the key.

This means your code should look like this:

<form
      onSubmit={event => {
        event.preventDefault();
        const id = event.target.elements[0].value;
        event.target.reset();

        // Adding state option along with navigation.
        navigate(`/invoices/${id}`, { state: {invoiceID: id }});
      }}
    >

I replaced

navigate(`/invoices/${id}`, { invoiceID: id });

with

navigate(`/invoices/${id}`, { state: {invoiceID: id }});

Hope that helps

@websymphony
Copy link
Author

Awesome, yes that was it. Many thanks!

@akoolenbourke
Copy link

This needs to be opened as a defect because the docs are just plain wrong - still. I spent hours trying to work this out only to eventually find this post that fixed it in an instant.

@jacekk
Copy link

jacekk commented Sep 6, 2018

There are at least two places that should be improved/fixed:

1) https://codesandbox.io/s/l27754822m

const Invoice = props => (
  <div>
    {console.log(props.location.state)} {/*Only has key, nothing else*/}
    <h2>Invoice {props.invoiceId}</h2>
  </div>
);

should be

const Invoice = props => (
  <div>
    {console.log(props.location.state)} {/* Has `key` and `invoiceID` keys */}
    <h2>Invoice {props.invoiceId}</h2>
  </div>
);

2) https://reach.tech/router/api/navigate

const NewTodo = () => (
  <TodoForm
    onSubmit={async todo => {
      let id = await createNewTodo(todo)
      // put some state on the location
      navigate("/todos", { newId: id })
    }}
  />
)

const Todos = props => (
  <div>
    {todos.map(todo => (
      <div
        style={{
          background:
            // read the location state
            todo.id === props.location.state.id
              ? "yellow"
              : ""
        }}
      >
        ...
      </div>
    ))}
  </div>
)

should be

const NewTodo = () => (
  <TodoForm
    onSubmit={async todo => {
      let id = await createNewTodo(todo)
      // put some state on the location
      navigate("/todos", { state: { newId: id }})
    }}
  />
)

const Todos = props => (
  <div>
    {todos.map(todo => (
      <div
        style={{
          background:
            // read the location state
            todo.id === props.location.state.newId
              ? "yellow"
              : ""
        }}
      >
        ...
      </div>
    ))}
  </div>
)

correct lines:

  • navigate("/todos", { state: { newId: id }}),
  • todo.id === props.location.state.newId.

I agree with @akoolenbourke that this very issue should be still opened.

@websymphony
Copy link
Author

The documentation was fixed via: #80

Problem is that the new documentation hasn't been deployed yet. We can keep this issue open until the documentation is updated.

@websymphony websymphony reopened this Sep 6, 2018
@akoolenbourke
Copy link

May I suggest docs fixes be deployed regularly. It's not like they need rigorous testing and it would really help.

I would also like better docs in general. As I'm quite new to react et al I found that the docs just assumed I knew a lot more than I did, without really pointing me to where to learn more. I had a nightmare trying to figure out why props.location didn't exist in one of my components until I found and I had to put that somewhere up the hierarchy to get props.location. Was a pain to be honest because some components have props.location and some don't and I really don't know why it is this way.

Note, I'm using Gatsby which uses @reach-router.

@rubenbase
Copy link

Please update the docs.

@postcasio
Copy link

I had this issue today. The documentation is still incorrect.

@trip16661
Copy link

Can confirm the docs are still incorrect.

https://reach.tech/router/api/navigate option section.

@b-tiwari
Copy link

please correct the wrong documentation, it costed me a day
https://reach.tech/router/api/navigate

@teseo
Copy link

teseo commented Jul 5, 2019

The documentation is still incorrect, please update https://reach.tech/router/api/navigate

@iifeoluwa
Copy link

Can confirm that I also lost some time to this today, kindly help fix this for others.

@mtliendo mtliendo added the needs docs Common questions that could be a guide in the docs, or missing API documentation label Sep 12, 2019
@mtliendo
Copy link

🎉docs have been updated to reflect the proper way to pass state into navigate. 🎉

@NicoJanvier
Copy link

I'm afraid the issue is still there:
https://reach.tech/router/api/navigate
navigate("/todos", { newId: id })

@jacekk
Copy link

jacekk commented Oct 26, 2019

@mtliendo @ryanflorence This issue has been fixed over a year ago in e8325ae - but still confuses people. May I ask why?

@RifRaf44
Copy link

RifRaf44 commented Nov 3, 2019

@mtliendo I'm afraid I ran into this issue today, luckily I stumbled on this link pretty quickly but the documentation is still showing the wrong syntax.

@mnishiguchi
Copy link

mnishiguchi commented Dec 8, 2019

@ryanflorence I checked the document's source to see if I can contribute. It is already fixed in the source markdown. It just has not been published yet for some reason.
https://github.com/reach/router/blob/master/website/src/markdown/api/navigate.md#option---state

@hassib-ashouri
Copy link

I think we should open another issue. docs are still wrong

@jamie29w
Copy link

Can you please re-open. The docs are still incorrect. Similar to @RifRaf44, I'm glad I found this issue.

Screen Shot 2020-02-13 at 2 17 36 PM

@jacekk
Copy link

jacekk commented Feb 13, 2020

@blainekasten , as the most active in this repo since the beginning of this year, can you take a look at this issue?

@blainekasten
Copy link
Collaborator

I'm pinging Ryan for this. I don't have website admin. I'll try to get this fixed

@blainekasten
Copy link
Collaborator

The docs have been updated! Sorry for the delay everone!

@odalysadam
Copy link

Hi!
So I'm doing this props.navigate('code', { state: { test: confirmationResult } }) and it navigates correctly, but state is null. Anyone any idea why?

@jacekk
Copy link

jacekk commented Mar 16, 2020

@odalysadam How do you "read" the state? An example code snippet would be helpful to answer.

@odalysadam
Copy link

@jacekk Of course, sorry. Currently just logging location out of props in the route component for /code. It shows the correct pathname as well, but it looks as though the whole app gets reloaded. ( app as in base path for my client side routes)

@jacekk
Copy link

jacekk commented Mar 16, 2020

@odalysadam I dunno why the whole app might gets realoded when using navigate, but please bear in mind that naviate('code') is a relative route. Maybe try /code... or maybe even just a slash /. Those are absolute routes - starting with a slash. Relative routes might be sometimes troublesome, especially when basePath is not the top-level one.

Just guessing though, as I've never had a problem with missing state while using navigate :/

@odalysadam
Copy link

@jacekk Thanks, I'll try an absolute path and see if the full reload still happens.

@odalysadam
Copy link

@jacekk Using an absolute path changed nothing, but state is most likely null because on reload everything gets lost.

Is triggering a full reload something that navigate even does or should I be looking elsewhere for the problem?

@jacekk
Copy link

jacekk commented Mar 18, 2020

@odalysadam It works for me:
Edit brave-turing-dtkcn

Can you replicate your case? Or share your project/code? Simple
props.navigate('/code', { state: { test: 'confirmationResult' } });
should be working correctly.

@odalysadam
Copy link

@jacekk Ok, then the problem lies elsewhere. Thank you very much!

@HenryKenya
Copy link

I can see the state, but it only has key...nothing more
navigate("/auth", { test: "test"})

Screenshot 2020-10-12 at 12 14 34 (2)

@jacekk
Copy link

jacekk commented Oct 12, 2020

I can see the state, but it only has key...nothing more
navigate("/auth", { test: "test"})

@HenryKenya Try this:

navigate("/auth", { state: { fooKey: "fooValue" }});

@HenryKenya
Copy link

@jacekk done that. Still having the same issue of seeing only keys

I can see the state, but it only has key...nothing more
navigate("/auth", { test: "test"})

@HenryKenya Try this:

navigate("/auth", { state: { fooKey: "fooValue" }});

@jacekk
Copy link

jacekk commented Oct 12, 2020

@HenryKenya Then try to replicate in some other component or in a separate project. This definitely works:
Edit brave-turing-dtkcn

@HenryKenya
Copy link

@jacekk checked out the sandbox. It has the same issue of just showing keys and no value

@HenryKenya
Copy link

@jacekk checked it out again. The issue must be somewhere else. Let me try debug it

@HenryKenya
Copy link

could the issue be that I am trying to access state using the useLocation hook?

@jacekk
Copy link

jacekk commented Oct 12, 2020

checked out the sandbox. It has the same issue of just showing keys and no value

@HenryKenya
You saaay WUT? Please, just point out what is NOT working here:
image

@HenryKenya
Copy link

checked out the sandbox. It has the same issue of just showing keys and no value

@HenryKenya
You saaay WUT? Please, just point out what is NOT working here:
image

The sandbox works..confirmed 😂
Still not sure what it is I have done wrong. Are props the only way to access the same or can someone use useLocation hook too?

@jacekk
Copy link

jacekk commented Oct 12, 2020

This line --> https://github.com/reach/router/blob/master/src/index.js#L69 gets location from history prop, but the following line does not pass it --> https://github.com/reach/router/blob/master/src/index.js#L43

@HenryKenya
I guess you've found a bug, but it is not related to this very ticket 😄 Please, report a separate one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs docs Common questions that could be a guide in the docs, or missing API documentation
Projects
None yet
Development

No branches or pull requests