search

peter-mouland / react-lego

React-lego : incrementally add more cool stuff to your react app
http://react-lego.herokuapp.com/
MIT License
414 stars 36 forks source link

set-router-context.js in redux branch does not properly handle routing #23

Open ngoctranfire opened 7 years ago

ngoctranfire commented 7 years ago

I'm not super sure, but I took a look at the set-router-context.js and actually tried setting this up with my own project, but I noticed that it looks like the routes we are passing into the match() function does not have a history object, and a route that has correctly been synced with the redux store.

peter-mouland commented 7 years ago

<RouterContext {...renderProps} /> Is where this history is passed (comes with renderProps).

I'm not sure what the error is you are trying to describe. Are you able to give some steps to reproduce it to help me out?

ngoctranfire commented 7 years ago

I'm currently not at my computer right now but if you take a look at it for example I think you called makeRoutes(store). However, your makeRoutes function currently doesn't take in any Params. Also, there was another issue and it seems to be happening on just a branch of mine. It might just be an error of mine but I can confirm later today

ngoctranfire commented 7 years ago

Okay, let me try to explain the problem that I am seeing. I actually looked at a few guides on how to set up router context on the server side. It seems that, for some reason, that your implementation differs quite a bit from the implementation I've seen from other repos. Firstly, I noticed that a lot of the other repos, noticeably, this repo: https://github.com/erikras/react-redux-universal-hot-example/blob/master/src/server.js. They actually grab the history, and then make sure its synced with the store before they pass it into match({}). Also, I noticed how there configureStore.js is much different from ours. For example, referring to this project again: https://github.com/erikras/react-redux-universal-hot-example/blob/master/src/redux/create.js They actually pass in the history to the store. We don't have any of this in our store at all. It seems each time we grab our routes, it is a brand new set of routes, with no prior history information.

Not sure if that made too much sense. I'm still kind of new to this, but just from seeing these implementations and comparing it with yours, it seems kind of off. I tried to use yours and it didn't work out so well for me in including it into my own project. I ended up with getting a lot hash routes when I wanted them to be browserHistory style routes.

peter-mouland commented 7 years ago

I've not had any problems with hash-history instead of browser-history. I haven't integrated `react-router-redux' (as they have in the example project); do you also use this?

I think react-router-redux and the passing around of the history into more places, is to enable time-travel ability (to replay user actions including route changes). I've not tried to implement this yet, but the prospect of being able to debug errors in this way is intriguing!

It does sound like a potential new branch (off the redux-dev-tools branch) to enable time-travel...