Clear shared redux stores when server rendering

[justin808]

Per #774 (comment), we should clear the redux stores before populating it.

The potential bug is that there are some stores with data from unrelated requests sitting around.

This can chew up memory and lead to other unexpected consequences.

Steps to fix:

1. Add a function in the JS library to clear all shared redux stores.
2. Call that function from https://github.com/shakacode/react_on_rails/blob/master/app/helpers/react_on_rails_helper.rb#L346, prepending this to the result.

See #774 for more details.

[justin808]

@udovenko indicated interest, so I'll flag this one as taken.

[udovenko]

@justin808 Not sure I see the way to reset the store. Each project has its own set of reducers and its own logic for bootstrapping the data to store. So resetting the store state should be individual for each JS server bundle.

[justin808]

Just clear the global map.

https://github.com/shakacode/react_on_rails/blob/master/node_package/src/StoreRegistry.js#L4

hydratedStores.clear() or something like that...

[udovenko]

You mean clear stores registration completely? Feels like we will loose some benefits form having initiated stores on the next request...

[justin808]

For the reasons you raised, we don't want any hydrated stores on the next call.

[udovenko]

Ok, I'll see what I can do. Can't promise anything for sure unfortunately...

[udovenko]

@justin808 I've forked them gem and tried to reproduce an issue. But it seems that ReactOnRails creates and re-registers Redux store before every component is renderd on the server. It allways calls storeGenerator(reduxProps, railsContext) on https://github.com/shakacode/react_on_rails/blob/master/app/helpers/react_on_rails_helper.rb#L345. Store generator returns new store created with combineReducers and createStore. So it seems store state cannot be shared across components render on the server.

To test it a bit more I added consoel.log('new store!') to https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/client/app/stores/SharedReduxStore.jsx#L12. Here is a console replay from server for http://0.0.0.0:5000/server_side_hello_world_shared_store:

[SERVER] new store!
[SERVER] RENDERED ReduxSharedStoreApp to dom node with id: ReduxSharedStoreApp-react-component-0 with railsContext: {"inMailer":false,"i18nLocale":"en","i18nDefaultLocale":"en","href":"http://0.0.0.0:5000/server_side_hello_world_shared_store","location":"/server_side_hello_world_shared_store","scheme":"http","host":"0.0.0.0","port":5000,"pathname":"/server_side_hello_world_shared_store","search":null,"httpAcceptLanguage":"ru-RU,ru;q=0.8,en-US;q=0.6,en;q=0.4","somethingUseful":"REALLY USEFUL","serverSide":true}
[SERVER] This is a script:"</div>"(/script> <script>alert('WTF1')(/script>
[SERVER] Script2:"</div>"(/script xx> <script>alert('WTF2')(/script xx>
[SERVER] Script3:"</div>"(/script xx> <script>alert('WTF3')(/script xx>
[SERVER] Script4"</div>"(/script <script>alert('WTF4')(/script>
[SERVER] Script5:"</div>"(/script> <script>alert('WTF5')(/script>
[SERVER] railsContext.serverSide is  truehttps://github.com/shakacode/react_on_rails/issues/774
[SERVER] new store!
[SERVER] RENDERED ReduxSharedStoreApp to dom node with id: ReduxSharedStoreApp-react-component-1 with railsContext: {"inMailer":false,"i18nLocale":"en","i18nDefaultLocale":"en","href":"http://0.0.0.0:5000/server_side_hello_world_shared_store","location":"/server_side_hello_world_shared_store","scheme":"http","host":"0.0.0.0","port":5000,"pathname":"/server_side_hello_world_shared_store","search":null,"httpAcceptLanguage":"ru-RU,ru;q=0.8,en-US;q=0.6,en;q=0.4","somethingUseful":"REALLY USEFUL","serverSide":true}
[SERVER] This is a script:"</div>"(/script> <script>alert('WTF1')(/script>
[SERVER] Script2:"</div>"(/script xx> <script>alert('WTF2')(/script xx>
[SERVER] Script3:"</div>"(/script xx> <script>alert('WTF3')(/script xx>
[SERVER] Script4"</div>"(/script <script>alert('WTF4')(/script>
[SERVER] Script5:"</div>"(/script> <script>alert('WTF5')(/script>
[SERVER] railsContext.serverSide is  true

Two components were rendered in the same js_context but store was recreated for each render. So it seems I can't reproduce the issue.

I think the case I've mentioned in #774 - "As I can remember I've even experienced an issue when store state from previous request was affecting next request. At least I thought that was the reason for my bug that time..." - was when I was creating the store manually from component generator function.

If you're insist that the issue still exists, can you please show how to reproduce it?

[justin808]

@udovenko For any stores being registered, they are 100% created new. I'm concerned that you could have a store that sticks around from the previous request. And the component rendering code might be accidentaly using that store by a different name.

So to reproduce:

1. Register 2 stores, A & B.
2. Run one request that uses store A
3. Run a second request that users store B. In the code that uses store B, print the contents of all stores. You could put in a debug line somewhere that prints out (at least the keys) of hydratedStores from this file.

If Store A's values are still there, then we have an issue.

[justin808]

Keep in mind that if one is careful to hydrate all the used stores for a given request, then no bug would ever be seen. However, there's certainly some extra memory that could be GC'd.

[udovenko]

Even if the store is stick around from previous request, https://github.com/shakacode/react_on_rails/blob/master/app/helpers/react_on_rails_helper.rb#L346 will replace it with new instance anyway. Am I missing someting?

[udovenko]

@justin808 Ok, I see what do you mean. The purpose for cleaning stores left from previous request is generally for GC only. Because they cannot affect components rendering.

[udovenko]

Or they can affect components rendering due to developer mistake...

[udovenko]

@justin808 PR #785