Implement in react-redux-universal-hot-example

[quicksnap]

There's some talk about this library in erikras/react-redux-universal-hot-example#439

Would you be interested in starting a branch to integrate this library as a replacement? It could serve as a good example on how to migrate from redux-router to redux-simple-router, and could uncover pain points along the way.

I'd be happy to help, too. But for the next few days, probably not much.

If you think it would not serve as a good replacement, that would be great information, too!

[jlongster]

That would be an interesting experiment, and I'm sure I'd learn a few things about why redux-router works the way it does. I don't think I'll have time to do it in the next few days (not really familiar with it) but I'll keep this issue open and maybe next week I can take a shot.

[quicksnap]

Totally--I'm interested too, but pretty busy myself. I haven't personally run into any issues with redux-router in my project, so it's low priority for my own needs.

[jlongster]

Alright, I did an initial port here: jlongster/react-redux-universal-hot-example@e13b935?diff=split

Who else should I loop in? @stevoland @erikras @justingreenberg? I'll play around with it for a few more days.

It's more elegant IMHO. Particularly server-rendering. No more hacks to store the data fetching promise on the state or anything like that. The state should stay pure. You can just use react-router straight up.

[quicksnap]

Looks good at first glance--thanks for doing this!

I'll try and take a look tomorrow and port it into my project.

[bendiy]

@jlongster Thanks for putting this together so quickly. I was struggling with redux-router and this approach seems much simpler.

I found a small change here:
jlongster/react-redux-universal-hot-example@e13b935?diff=split#commitcomment-14335312

[Dattaya]

@jlongster dispatching during this stage gave me this warning on the client while transitioning: Warning: setState(...): Cannot update during an existing state transition (such as within render). Render methods should be a pure function of props and state. What about you, did you test your code?

[erikras]

If you guys could kick the tires on this a bit more, I'd love to merge it.

[quicksnap]

I definitely think we should test this out for about a week before merging, just to make sure! But looks great.

[jlongster]

Will kick around some more soon, been a busy week at work. @Dattaya thanks for the report.

[louissmit]

I think fetchDataDeferred stopped working in @jlongster 's port? Like when you route to /widgets

[stevoland]

@louissmit I think fetchData becomes the equivalent of fetchDataDeferred as it no longer blocks rendering of the container: jlongster/react-redux-universal-hot-example@e13b935?diff=split#diff-cf27c1d543e886c89cd9ac8b8aeaf05bR39 This is fine as far as I'm concerned.

Looks great! I'm starting a project right now and this is going in for sure.

[louissmit]

@stevoland I'm not sure I see how it no longer blocks, but in either case the widgets example is broken in its current implementation.

I'm new to this boilerplate and redux so I'm just trying to figure out how it works. (:

[jlongster]

@louissmit haven't had a chance to flesh all this out yet, this week has been busy. I'm going to work on it some this weekend.

On the client-side, it does not block, but that's just the strategy I chose for now. It we want it to block to emulate the previous behavior, we just need to use onEnter which has a 3rd callback argument.

[quicksnap]

Glad to hear @stevoland likes this! I think I'll have some time today to try it. My project relies on both fetchData and fetchDataDeferred, so if I can get it working I'd be stoked!

[Dattaya]

Dispatching in createElement might not be a good way of doing things. @timdorr hasn't written anything here yet (I asked him to give some advice), but this is what he recommended, how he implemented data pre-fetch on the client https://gist.github.com/timdorr/3ffe30e3c4e116019bc3 this is one way to implement fetchDataDeferred, not sure about fetchData on the client.

[timdorr]

Note that there are a few libraries out there for the exact same thing, such as react-fetcher and react-resolver.

[justingreenberg]

thanks @timdorr react-fetcher looks like a good fit, actually the author @markdalgleish has already submitted a PR for the redux-router implementation of erikras/react-redux-universal-hot-example#531 which is in a holding pattern while this PR is tested

[timdorr]

The biggest thing react-fetcher needs is the ability to also run on componentDidMount, so you don't have to duplicate your fetching logic when mounting a component (such as when entering a route) inside of each component. Other than that, it's a pretty good start for a 0.2.0 version.

[quicksnap]

Just gave it a whirl:

• I am aslo getting the setState() warnings during transition.
• Before we merge, I feel there should be a good pattern for a route-blocking fetchData. There are good use cases for it.

I'm not sure where the setState bug is. I may have some time to investigate later.

[quicksnap]

More information: the setState() is being called within @connect, which is being called from RoutingContext render().

Per the warning, setState within render is bad.

[quicksnap]

As a hack, wrap fetchData in a timeout: https://github.com/jlongster/react-redux-universal-hot-example/pull/2/files#diff-cf27c1d543e886c89cd9ac8b8aeaf05bR41

[quicksnap]

As for the issue with blocking route transitions, would it be sensible to create a Route HOC that checks for a static method within onEnter?

[markdalgleish]

@timdorr react-fetcher is a relatively small library, and it doesn't place any restrictions on when you can fetch data. There's no reason you couldn't do it on componentDidMount if you want. In fact, v0.2.0 was released in anticipation of making this use case easier by allowing data fetching for a single component, not just arrays of components.

[sompylasar]

@jlongster there are a couple of serious errors in the example implementation, please see my comments:
jlongster/react-redux-universal-hot-example@e13b935?diff=split#commitcomment-14403049
jlongster/react-redux-universal-hot-example@e13b935?diff=split#commitcomment-14403070

[jlongster]

I've been moving forward slowly with this project because I really wanted to think some things though. This PR implements a very different way to sync route changes: #40

It looks like that works well, and I still want to add a test suite to this project. I wanted to wait until this stabilizes a bit before fully porting that project. It will be soon though.

[glennr]

@sompylasar - not sure if my question on jlongster/react-redux-universal-hot-example@e13b935?diff=split#commitcomment-14403070 will come through - so here it is - how do we reproduce the server-hang-on-rejected-promise issue. I had little luck reproducing it.

[glennr]

And another issue not-related-to-this-project-but-to-jlongster/react-redux-universal-hot-example...

@quicksnap - re: the setState() issue - did you notice the same problem on the Chat component?

https://monosnap.com/file/ouGicqqm47vq3YIqiqT2aNYEsLgbFD.png

[sompylasar]

@glennr replied there.

[quicksnap]

@glennr I ran this in my own project based off the boilerplate, so Chat isn't present. However, the issue will be present in various states.

[Ghostavio]

hey guys
how is the progress on this?

thanks :)

[tomatau]

Shameless plug: I have a universal boilerplate application that's working with redux-simple-router and koa. https://github.com/tomatau/breko-hub - the integration seems to work consistently and takes place in src/app/state/store.js.

Any criticism on the approach taken is more than welcome!

[capkutay]

I also noticed Chat isn't in this version of the boilerplate. Is there a reason why it was excluded?

[bdefore]

Not sure what the timeline is like for this, but I made a PR bringing much of this and its open pull requests into example project, along with universal-redux npm package.

That PR is here: erikras/react-redux-universal-hot-example#685

[mwildehahn]

getting the setState error on the client as well:

Warning: setState(...): Cannot update during an existing state transition (such as within `render`). Render methods should be a pure function of props and state.```

[mwildehahn]

I followed the structure you recommended for server side rendering, ie. if the component has a fetchData static method, which is called in a custom createElement function that gets passed to Router. One issue I'm running into is that the fetchData call will be executed before react life cycle methods.

Specifically, I have a search component that clears its cache within componentWillUnmount, lets call it component A. There is a scenario where we load another component (component B) which fetches search results in its fetchData.

what is happening now is:

as we transition from A to B:

• B's fetchData is called
• A's componentWillUnmount is called
• B's componentWillMount is called

this is breaking for me because fetchData thinks it has cached search results, which get cleared in componentWillUnmount.

If i fetch the results in component B's componentWillMount i'm triggering multiple search requests on the server side. It isn't a huge deal since only 1 hits the API since the subsequent one in componentWillMount uses the cache, but it is a little messy.

[mariocasciaro]

Somebody might find it useful to take a look at the code in this PR as well as the associated discussion: jlongster/react-redux-universal-hot-example#4

[oyeanuj]

As someone who was enthusiastically following the redux-simple-router, just checking in if there are any updates here and if it would play well with the react-router 2.0? :) @jlongster @mariocasciaro

[jlongster]

We're working on a new API in #141 and just landed it. Basically we're still settling on the right solution so I'm not focusing on any template/boilerplate project yet until we've stabilized. I think what's on master might be pretty robust, but we'll see.

[timdorr]

Is this still being pursued? 2.0 is out, so now's the time!

[bdefore]

@timdorr I believe this issue would still be problematic for RRUHE: jlongster/react-redux-universal-hot-example#4

I've managed to bring version 2.0.1 (and react-router 2.0.0-rc5) into universal-redux 3.0.0-rc7 (https://github.com/bdefore/universal-redux). It's working like a charm there using async-props rather than the serverside fetching mechanism of RRUHE.

[timdorr]

I'm going to close this, as it can either be tracked on @jlongster's repo or the original. Either way, outside of specific fixes the library has to make (which would require issues here), there isn't really anything to do in this repo.

[Dattaya]

I like proposed solutions to this problem. There is another one--bring back universalRouter (see universalRouter, client.js, server.js.)
It would be nice if someone knowledgeable about react-router and history advise us whether using match this way on the client is a good design or a hacky solution that can lead to bugs/complications in the future.
I tried to improve it to handle errors identically. This is a basic idea: https://gist.github.com/Dattaya/191bc290ed1bc7855e33

Here're few potential benefits:

• symmetry between the client and the server which is an indication of a good design.
• it's relatively simple (66 lines of code)
• better error handling. For now it handles 5xx errors and for others returns 404 page. (it has a downside, preload can't be set to false)

If you want to see error handling in action, take a look at my repository https://github.com/Dattaya/isomorphic-redux-plus/tree/universal-router I use it because it's dead simple which means it's much easier to experiment with + hot-reloading is blazingly fast.
API is configured to return http error 500 if id === 5. Error page could be seen only in production, in the development mode it just calls reject and prints error message to the console.
Run npm run build, npm start. Open http://localhost:3000/todos. If you press on Todo item 5, you'll see an error page: 500 Internal Server Error Administrators have been notified. Please try again later. If you load it directly http://localhost:3000/todos/5 , the server will respond with the same page; in addition the status of the page will be 500.

[quicksnap]

Update on this: we have an experimental branch, simple-router here: https://github.com/erikras/react-redux-universal-hot-example/tree/simple-router

IMO, it's best to discuss ideas in RRUHE repo if it doesn't directly relate to react-router-redux so not to drain on the collaborators here.

[Dattaya]

I'll open then a new issue in RRUHE sometime this week.