Browser's memory keeps increasing when using React-engine & React-router

[nguyenhhien]

I'm writing Isormophic Web Application by using React-engine & React-router. Before upgrading, my application is running well with the following configuration.
"react": "^0.14.8",
"react-dom": "^0.14.6",
"react-engine": "^3.4.1",
"react-router": "^1.0.3"
Recently, I've upgraded my system to use official React with the following configuration, and I observed my app become slower and slower when running on browser after every click I made on ReactRouter.Link. I've opened Google Chrome Debugger for investigation, then I see usage memory keep increasing after every click.
"react": "^15.2.1",
"react-dom": "^15.2.1",
"react-engine": "^4.0.0",
"react-router": "^2.5.2"
Have anyone faced the similar problem? Please help me. Thank you :)

[ghost]

Having the same issue here. The render() method on the outermost layout keeps getting called. The example has the html tag in the outermost react-router layout route. I've never seen it done like that before; won't that cause the entire DOM to get re-rendered on every route switch and leave all the bindings orphaned?

I added more markup to the detail view of the example app and a Router.Link back to the list view. After 10 or so route changes (each of which call render() on the outer layout) the app starts slowing down and the memory footprint of the app in the browser continues to grow. This pattern (rendering the entire DOM from react-router) seems unworkable for anything but really tiny apps with minimal data binding or route nesting.

[JustinMGaiam]

We are encountering an issue with FireFox and IE where they become unusable quickly as our Immutable data in Redux grows after a few clicks which appears to be from out of control memory growth triggering rapid garbage collection. This does not happen in Chrome or Safari, so far I have tracked this down to lib/client.js line 97 https://github.com/paypal/react-engine/blob/master/lib/client.js#L97 does this look like the issue to anyone else?

If you change from lodash merge to lodah assign the issue seems to go away does this work for anyone else? Is there a reason _.merge was used over _.assign? Since our data is all Immutable.js the data is much bigger then standard object or array's so I think merge is going too deep when assign I believe is much more shallow. Let me know if the move to _.assign makes sense and I can do a super quick PR.

Also note that the route change speed is night and day with the switch to _.assign.

[JustinMGaiam]

If anyone wants to merge the PR it is here #169 my only thought about this not working would be if the router props were some how the same name as the component props the router props would win instead of being merge together. Not sure anyone would expect two component with props of the same name to combine into a new "super" prop, but they would lose that with this change.

[SOSANA]

@JustinMGaiam you have an example of react-engine redux implentation I could checkout?

[samsel]

@nguyenhhien @keithchilders can you try 4.0.1 with @JustinMGaiam's fix?

[JustinMGaiam]

@SOSANA kraken-react-engine-router-redux-webpack is not my project, but it looks similar to how we came to our implementation since React Engine kind of forces you into attaching Redux to your high level app components since it cannot be attached to the Router props. To make this project closer to how we are using it you would need to hook up all of the Redux store using ImmutableJS and not plain Javascript object and arrays. When you are using ImmutableJS it does not take too much data to start seeing issues with merge if you are in IE or FireFox. Safari, Chrome, and Even Opera seem to do much better with processing very large arrays of data.

https://github.com/chunkiat82/kraken-react-engine-router-redux-webpack

[SOSANA]

@JustinMGaiam Thank you for replying back, much appreciated. I am using express but waiting for a fix for pr for #170 to fix an issue using provider with redux. That repo had me curious using kraken for future projects. Cheers!

[JustinMGaiam]

@SOSANA check out #172 this is the same fix for the server as I applied to the client. Works equally as well and I missed it the first time around.

[SOSANA]

@JustinMGaiam Look forward to #172 merge 👍 in few days need to wire up react-engine with redux provider which I believe #171 merge now supports as well