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

Remove withRouter #21

Merged
merged 1 commit into from
Sep 22, 2016
Merged

Remove withRouter #21

merged 1 commit into from
Sep 22, 2016

Conversation

josephsavona
Copy link
Contributor

What is withRouter? In general it's helpful to introduce as few new concepts at a time. I already know how Relay works, and I was immediately confused by withRouter, so I imagine people reading will be too.

What is `withRouter`? In general it's helpful to introduce as few new concepts at a time. I already know how Relay works, and I was immediately confused by `withRouter`, so I imagine people reading will be too.
@schickling
Copy link
Collaborator

I agree! @Brene / @lvarayut is this needed here?

@lvarayut
Copy link
Member

Totally agree with Joseph. I don't think we need it.

@schickling
Copy link
Collaborator

@lvarayut we just need to make sure it's aligned with the working example project.

@marktani
Copy link
Member

I remember @Brene having a distinct reason for including withRouter. In any case I agree that at the moment having it here just introduces a lot of confusion. I think we can add a sentence or two on withRouter at this step, and explain it further down the road in the Router chapter.

Would love to hear @Brene's opinion on that :)

@renebrandel
Copy link
Collaborator

renebrandel commented Sep 20, 2016

I'm a bit overwhelmed today. I'll look over it tomorrow ;-)

@renebrandel
Copy link
Collaborator

@josephsavona seems to be right. It looks like some "old" legacy code and we should remove it from all the examples. Previously we used this.props.router.push('...'). But I've replaced everything with Link. That's the issue. @marktani do you have time to change it in the pokedex example?

@renebrandel renebrandel merged commit 184a303 into learnrelay:master Sep 22, 2016
@marktani
Copy link
Member

Ok will do that 👍

@alexedev
Copy link

withRouter is still used starting from these examples https://learnrelay.org/queries/working-with-fragments

@marktani
Copy link
Member

Thanks for pointing this out, fixed it in 294f4d1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants