Clear hydrated stores

[udovenko]

• Create clearHydratedStores() methods for StoreRegistry.js and ReactOnRails.js
• Add clearHydratedStores() invocation to #initialize_redux_stores helper
• Add JS tests

---

This change is 

[coveralls]

Coverage increased (+0.003%) to 97.922% when pulling c0b43b2 on udovenko:clear_hydrated_stores into 85a79c4 on shakacode:master.

[justin808]

Test needs work. Looking promising!

---

Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

---

node_package/src/ReactOnRails.js, line 45 at r1 (raw file):

  /**
   * Clears hydratedStores to avoid accidental usage of wrong store hydrated in previous/parallel

1. There's no parallel requests, with the connection pool.
2. This should be down below in the place with the comment HERE, b/c this is internal

---

node_package/src/ReactOnRails.js, line 144 at r1 (raw file):

// /////////////////////////////////////////////////////////////////////////////
// INTERNALLY USED APIs
// /////////////////////////////////////////////////////////////////////////////
 
HERE for clearHydratedStores() {

---

node_package/tests/ReactOnRails.test.js, line 138 at r1 (raw file):

  }

  ReactOnRails.registerStore({ storeGenerator });

registration is what user code does to tell React on Rails the store's code.

We need to be testing setStore with some value passed in.

 /**
   * Internally used function to set the hydrated store after a Rails page is loaded.
   * @param name
   * @param store (not the storeGenerator, but the hydrated store)
   */
  setStore(name, store) {
    hydratedStores.set(name, store);
  },

---

node_package/tests/StoreRegistry.test.js, line 102 at r1 (raw file):

  StoreRegistry.clearHydratedStores();
  const expected = new Map();
  assert.deepEqual(StoreRegistry.stores(), expected);

see comments for ReactOnRails.test.js

---

Comments from Reviewable

[udovenko]

As I already showed in #774, there are parallel requests with ConnectionPool...Will fix other notices.

[coveralls]

Coverage increased (+0.003%) to 97.922% when pulling 66aca37 on udovenko:clear_hydrated_stores into 85a79c4 on shakacode:master.

[justin808]

One tiny comment and we need the CHANGELOG entry.

---

Reviewed 3 of 3 files at r2.
Review status: all files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

---

node_package/src/ReactOnRails.js, line 139 at r2 (raw file):

  /**
   * Clears hydratedStores to avoid accidental usage of wrong store hydrated in previous/parallel

no parallel requests

---

Comments from Reviewable

[udovenko]

@justin808 It is not a problem to remove word "parallel" form the comment, but could you please tell me why there are "no parallel requests"? I thought in #774 I've showed you that connection can be returned to the pool and any parallel request can check out this connection before current request is finished.

[justin808]

@udovenko Each connection (really a JS context) can be served from the pool to a thread for running a snippet of JS code. The number of these "connections" can be configured. They do not, as far as I know, share the same context. Thus, only ONE thread at a time has access to the given connection (js context).

Does that sound about right?

[udovenko]

@justin808 Yes you're right. Two threads do not share same js_context at the same moment but same js_context can be used by 2 parallel requests in turn. E.g. request A created stores 1 and 2 and returned connection to the pool. Request B started and have acces to both stores in case if it checked out the same js_context form the pool. By parallel requsts I mean requests to the web-server. Please look at the code snippet I've posted in #774.

[justin808]

@udovenko, great explanation. We just had different ideas of the potential concurrency issue. I'll merge as-is.

[udovenko]

@justin808 I'm not insisting on leaving this comment. Just wanted to clarify...

[justin808]

@udovenko Since I merged this, the codeship builds are failing:

These are the failing tests:

https://app.codeship.com/projects/187011/builds/23942593

Failed examples:

rspec ./spec/features/integration_spec.rb[15:1:1] # 2 react components, 1 store, server side /server_side_hello_world_shared_store Type in one component changes the other component
rspec ./spec/features/integration_spec.rb[17:1:1] # 2 react components, 1 store, server side, controller setup /server_side_hello_world_shared_store_controller Type in one component changes the other component
rspec ./spec/features/integration_spec.rb[19:1:1] # 2 react components, 1 store, server side, defer /server_side_hello_world_shared_store_defer Type in one component changes the other component
rspec ./spec/features/rails_context_spec.rb[1:2:1:1:1] # rails_context server rendering shared store server_side_hello_world_shared_store check rails context
rspec ./spec/requests/server_render_check_spec.rb:14 # Server Rendering generates server rendered HTML if server renderering enabled for shared redux
rspec ./spec/requests/server_render_check_spec.rb:108 # Server Rendering server rendering railsContext shared redux store matches expected values

What's odd is that Travis is passing.

[udovenko]

@justin808 Is this issue resolved now? I can't open the link you've provided to see what exactly went wrong. And I have no expirience with CodeShip... But before opening PR I launched both JS and RSpect tests two times and they all were green.

[justin808]

Hi @udovenko. I fixed the problem. It was due to the way that codeship was caching npm modules. Thanks for checking back with me.