Skip to content

Commit

Permalink
Fix dom_id collisions by using a UUID rather than an autoincremente…
Browse files Browse the repository at this point in the history
…d index.

This ensures that all DOM nodes ids are unique when multiple instances of a component co-exist on the same page.
It also allows fragment caching of generated HTML (since the unicity does not depend on other components on the page).

:warning: one must use the `id` parameter of `react_component` to force the generated DOM node id in order to have a predictable one (_e.g._ in tests).

This commit also includes the CHANGELOG update and the version bump.

Solves shakacode#437
  • Loading branch information
michaelbaudino committed Jun 1, 2016
1 parent c7b9209 commit ea3c7b3
Show file tree
Hide file tree
Showing 24 changed files with 78 additions and 91 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. Items under
Contributors: please follow the recommendations outlined at [keepachangelog.com](http://keepachangelog.com/). Please use the existing headings and styling as a guide, and add a link for the version diff at the bottom of the file. Also, please update the `Unreleased` link to compare to the latest release version.
## [Unreleased]

## [6.0.2]
##### Fixed
- Fix colisions in ids of DOM nodes generated by `react_component` by indexing in using an UUID rather than an auto-increment value. This means that it should be overriden using the `id` parameter of `react_component` if one wants to generate a predictable id (_e.g._ for testing purpose). See [Issue #437](https://github.com/shakacode/react_on_rails/issues/437). Fixed in [#438](https://github.com/shakacode/react_on_rails/pull/438) by [Michael Baudino](https://github.com/michaelbaudino).

## [6.0.1]
##### Fixed
- Allow for older version of manifest.json for older versions of sprockets. See [Issue #435](https://github.com/shakacode/react_on_rails/issues/435). Fixed in [#436](https://github.com/shakacode/react_on_rails/pull/436) by [alleycat-at-git](https://github.com/alleycat-at-git).
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ react_component(component_name,
+ **options:**
+ **props:** Ruby Hash which contains the properties to pass to the react object, or a JSON string. If you pass a string, we'll escape it for you.
+ **prerender:** enable server-side rendering of component. Set to false when debugging!
+ **id:** Id for the div. This will get assigned automatically if you do not provide an id.
+ **id:** Id for the div, will be used to attach the React component. This will get assigned automatically if you do not provide an id. Must be unique.
+ **html_options:** Any other html options to get placed on the added div for the component.
+ **trace:** set to true to print additional debugging information in the browser. Defaults to true for development, off otherwise. Note, on the client you will so both the railsContext and your props. On the server, you only see the railsContext being logged.
+ **replay_console:** Default is true. False will disable echoing server-rendering logs to the browser. While this can make troubleshooting server rendering difficult, so long as you have the default configuration of logging_on_server set to true, you'll still see the errors on the server.
Expand Down
11 changes: 1 addition & 10 deletions app/helpers/react_on_rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,8 @@ def react_component(component_name, raw_options = {})
# Create the JavaScript setup of the global to initialize the client rendering
# (re-hydrate the data). This enables react rendered on the client to see that the
# server has already rendered the HTML.
# We use this react_component_index in case we have the same component multiple times on the page.

react_component_index = next_react_component_index
options = ReactOnRails::ReactComponent::Options.new(name: component_name,
index: react_component_index,
options: raw_options)
options = ReactOnRails::ReactComponent::Options.new(name: component_name, options: raw_options)

# Setup the page_loaded_js, which is the same regardless of prerendering or not!
# The reason is that React is smart about not doing extra work if the server rendering did its job.
Expand Down Expand Up @@ -253,11 +249,6 @@ def render_redux_store_data(redux_store_data)
prepend_render_rails_context(result)
end

def next_react_component_index
@react_component_index ||= -1
@react_component_index += 1
end

def props_string(props)
props.is_a?(String) ? props : props.to_json
end
Expand Down
11 changes: 4 additions & 7 deletions lib/react_on_rails/react_component/options.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,8 @@ class Options
NO_PROPS = {}.freeze
HIDDEN = "display:none".freeze

attr_reader :index

def initialize(name:, index:, options:)
def initialize(name:, options:)
@name = name
@index = index
@options = options
end

Expand All @@ -21,7 +18,7 @@ def name
end

def dom_id
options.fetch(:id) { default_dom_id }
@dom_id ||= options.fetch(:id) { generate_unique_dom_id }
end

def html_options
Expand Down Expand Up @@ -62,8 +59,8 @@ def style

attr_reader :options

def default_dom_id
"#{@name}-react-component-#{@index}"
def generate_unique_dom_id
"#{@name}-react-component-#{SecureRandom.uuid}"
end

def retrieve_key(key)
Expand Down
2 changes: 1 addition & 1 deletion lib/react_on_rails/version.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# frozen_string_literal: true
module ReactOnRails
VERSION = "6.0.1".freeze
VERSION = "6.0.2".freeze
end
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "react-on-rails",
"version": "6.0.1",
"version": "6.0.2",
"description": "react-on-rails JavaScript for react_on_rails Ruby gem",
"main": "node_package/lib/ReactOnRails.js",
"directories": {
Expand Down
4 changes: 2 additions & 2 deletions spec/dummy/app/views/pages/client_side_hello_world.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<%= render "header" %>
<%= react_component("HelloWorld", props: @app_props_server_render, prerender: false, trace: true) %>
<%= react_component("HelloWorld", props: @app_props_server_render, prerender: false, trace: true, id: "HelloWorld-react-component-0") %>
<hr/>

<h1>React Rails Client Side Only Rendering</h1>
Expand Down Expand Up @@ -31,7 +31,7 @@
Place the component on the view: spec/dummy/app/views/pages/client_side_hello_world.html.erb
<br/>
<pre>
<%%= react_component("HelloWorld", props: @app_props_server_render, prerender: false, trace: true) %>
<%%= react_component("HelloWorld", props: @app_props_server_render, prerender: false, trace: true, id: "HelloWorld-react-component-0") %>
</pre>
</li>
</ol>
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
<%= render "header" %>
<%= redux_store("SharedReduxStore", props: @app_props_server_render) %>

<%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-0") %>
<hr/>
<h1>Second Hello World</h1>
<%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-1") %>
<hr/>

<h1>React Rails Client Side Only Rendering, 2 components, same Redux store </h1>
Expand All @@ -30,10 +30,10 @@
<pre>
<%%= redux_store("SharedReduxStore", props: @app_props_server_render) %>

<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-0") %>

Second Hello World
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-1") %>
</pre>
</li>
</ol>
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
<%= render "header" %>

<%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-0") %>
<hr/>
<h1>Second Hello World</h1>
<%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-1") %>
<hr/>

<h1>React Rails Client Side Only Rendering, 2 components, same Redux store </h1>
Expand All @@ -29,10 +29,10 @@
<pre>
<%%= redux_store("SharedReduxStore", props: @app_props_server_render) %>

<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-0") %>

Second Hello World
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-1") %>
</pre>
</li>
</ol>
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
YO
<% redux_store("SharedReduxStore", props: @app_props_server_render, defer: true) %>

<%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-0") %>
<hr/>
<h1>Second Hello World</h1>
<%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-1") %>
<hr/>

<h1>React Rails Client Side Only Rendering, 2 components, same Redux store </h1>
Expand All @@ -32,10 +32,10 @@ YO
<pre>
<%%= redux_store("SharedReduxStore", props: @app_props_server_render) %>

<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-0") %>

Second Hello World
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-1") %>
</pre>
</li>
</ol>
2 changes: 1 addition & 1 deletion spec/dummy/app/views/pages/client_side_log_throw.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<%= render "header" %>

<%= react_component("HelloWorldWithLogAndThrow", props: @app_props_server_render,
prerender: false, trace: true) %>
prerender: false, trace: true, id: "HelloWorldWithLogAndThrow-react-component-0") %>
<hr/>

<h1>React Rails Client Rendering</h1>
Expand Down
24 changes: 12 additions & 12 deletions spec/dummy/app/views/pages/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ This page demonstrates a few things the other pages do not show:

<pre>
<%% cache @app_props_server_render do %><br/>
<%%= react_component("ReduxApp", props: @app_props_server_render, trace: true) %><br/>
<%%= react_component("ReduxApp", props: @app_props_server_render, trace: true, id: "ReduxApp-react-component-0") %><br/>
<%% end %>
</pre>
<p>
Expand All @@ -36,46 +36,46 @@ This page demonstrates a few things the other pages do not show:
<% puts "server rendering react components" %>
<% puts "=" * 80 %>
<% end %>
<%= react_component("ReduxApp", props: @app_props_server_render, trace: true) %>
<%= react_component("ReduxApp", props: @app_props_server_render, trace: true, id: "ReduxApp-react-component-0") %>
<hr/>

<h1>Server Rendered/Cached React Component Without Redux</h1>
<pre>
<%% cache @app_props_server_render do %><br/>
<%%= react_component("HelloWorld", props: @app_props_server_render, trace: true) %><br/>
<%%= react_component("HelloWorld", props: @app_props_server_render, trace: true, id: "HelloWorld-react-component-1") %><br/>
<%% end %>
</pre>
<p>
And this is an example of a server rendered React component without Redux.
Note, that we don't suffix the component name with "App"
</p>

<%= react_component("HelloWorld", props: @app_props_server_render, trace: true) %>
<%= react_component("HelloWorld", props: @app_props_server_render, trace: true, id: "HelloWorld-react-component-1") %>
<% end %>
<hr/>

<h1>Simple Client Rendered Component</h1>
<!-- Passing prerender: false to not render on server -->
<pre>
<%%= react_component("HelloWorldApp", props: @app_props_hello, prerender: false, trace: true) %>
<%%= react_component("HelloWorldApp", props: @app_props_hello, prerender: false, trace: true, id: "HelloWorldApp-react-component-2") %>
</pre>
<%= react_component("HelloWorldApp", props: @app_props_hello, prerender: false, trace: true) %>
<%= react_component("HelloWorldApp", props: @app_props_hello, prerender: false, trace: true, id: "HelloWorldApp-react-component-2") %>
<hr/>

<h1>Showing you can put the same component twice on a page with different props</h1>
<pre>
<%%= react_component("HelloWorldApp", props: @app_props_hello_again, prerender: false, trace: true) %>
<%%= react_component("HelloWorldApp", props: @app_props_hello_again, prerender: false, trace: true, id: "HelloWorldApp-react-component-3") %>
</pre>
<%= react_component("HelloWorldApp", props: @app_props_hello_again, prerender: false, trace: true) %>
<%= react_component("HelloWorldApp", props: @app_props_hello_again, prerender: false, trace: true, id: "HelloWorldApp-react-component-3") %>
<hr/>

<h1>Simple Component Without Redux</h1>
<pre>
<%%= react_component("HelloWorld", props: @app_props_hello, prerender: false, trace: true) %>
<%%= react_component("HelloES5", props: @app_props_hello, prerender: false, trace: true) %>
<%%= react_component("HelloWorld", props: @app_props_hello, prerender: false, trace: true, id: "HelloWorld-react-component-4") %>
<%%= react_component("HelloES5", props: @app_props_hello, prerender: false, trace: true, id: "HelloES5-react-component-5") %>
</pre>
<%= react_component("HelloWorld", props: @app_props_hello, prerender: false, trace: true) %>
<%= react_component("HelloWorldES5", props: @app_props_hello, prerender: false, trace: true) %>
<%= react_component("HelloWorld", props: @app_props_hello, prerender: false, trace: true, id: "HelloWorld-react-component-4") %>
<%= react_component("HelloWorldES5", props: @app_props_hello, prerender: false, trace: true, id: "HelloWorldES5-react-component-5") %>
<hr/>

<h1>Non-React Component</h1>
Expand Down
5 changes: 3 additions & 2 deletions spec/dummy/app/views/pages/pure_component.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<%= render "header" %>

<%= react_component("PureComponent", props: { title: "This is a Pure Component!" }, prerender: true, trace: true) %>
<%= react_component("PureComponent", props: { title: "This is a Pure Component!" }, prerender: true, trace: true, id: "PureComponent-react-component-0") %>

<hr/>

Expand All @@ -9,5 +9,6 @@
<%%= react_component("PureComponent",
{ title: "This is a Pure Component!" },
prerender: true,
trace: true) %>
trace: true,
id: "PureComponent-react-component-0") %>
</pre>
4 changes: 2 additions & 2 deletions spec/dummy/app/views/pages/server_side_hello_world.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<%= render "header" %>

<%= react_component("HelloWorld", props: @app_props_server_render, prerender: true, trace: true) %>
<%= react_component("HelloWorld", props: @app_props_server_render, prerender: true, trace: true, id: "HelloWorld-react-component-0") %>
<hr/>

<h1>React Rails Server Rendering</h1>
Expand Down Expand Up @@ -66,7 +66,7 @@
Place the component on the view: spec/dummy/app/views/pages/client_side_hello_world.html.erb
<br/>
<pre>
<%%= react_component("HelloWorld", props: @app_props_server_render, prerender: true, trace: true) %>
<%%= react_component("HelloWorld", props: @app_props_server_render, prerender: true, trace: true, id: "HelloWorld-react-component-0") %>
</pre>
</li>
</ol>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<%= render "header" %>
<%= react_component("HelloWorldES5", props: @app_props_server_render, prerender: true, trace: true) %>
<%= react_component("HelloWorldES5", props: @app_props_server_render, prerender: true, trace: true, id: "HelloWorldES5-react-component-0") %>
<hr/>
<h1>React Rails Server Rendering ES5 Style React.createClass</h1>
<p>
Expand Down Expand Up @@ -34,7 +34,7 @@
Place the component on the view: spec/dummy/app/views/pages/client_side_hello_world.html.erb
<br/>
<pre>
<%%= react_component("HelloWorldES5", props: @app_props_server_render, prerender: true, trace: true) %>
<%%= react_component("HelloWorldES5", props: @app_props_server_render, prerender: true, trace: true, id: "HelloWorldES5-react-component-0") %>
</pre>
</li>
</ol>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
<%= render "header" %>
<%= redux_store("SharedReduxStore", props: @app_props_server_render) %>

<%= react_component("ReduxSharedStoreApp", prerender: true, trace: true) %>
<%= react_component("ReduxSharedStoreApp", prerender: true, trace: true, id: "ReduxSharedStoreApp-react-component-0") %>
<hr/>
<h1>Second Hello World</h1>
<%= react_component("ReduxSharedStoreApp", prerender: true, trace: true) %>
<%= react_component("ReduxSharedStoreApp", prerender: true, trace: true, id: "ReduxSharedStoreApp-react-component-1") %>
<hr/>

<h1>React Rails Client Side Only Rendering, 2 components, same Redux store </h1>
Expand All @@ -30,10 +30,10 @@
<pre>
<%%= redux_store("SharedReduxStore", props: @app_props_server_render) %>

<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-0") %>

Second Hello World
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-1") %>
</pre>
</li>
</ol>
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
<%= render "header" %>

<%= react_component("ReduxSharedStoreApp", prerender: true, trace: true) %>
<%= react_component("ReduxSharedStoreApp", prerender: true, trace: true, id: "ReduxSharedStoreApp-react-component-0") %>
<hr/>
<h1>Second Hello World</h1>
<%= react_component("ReduxSharedStoreApp", prerender: true, trace: true) %>
<%= react_component("ReduxSharedStoreApp", prerender: true, trace: true, id: "ReduxSharedStoreApp-react-component-1") %>
<hr/>

<h1>React Rails Client Side Only Rendering, 2 components, same Redux store from controller</h1>
Expand Down Expand Up @@ -36,10 +36,10 @@ redux_store("SharedReduxStore", props: @app_props_server_render)
Place the components: spec/dummy/app/views/pages/server_side_hello_world_shared_store_controller.html.erb
<br/>
<pre>
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-0") %>

Second Hello World
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-1") %>
</pre>
</li>
</ol>
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
<%= render "header" %>
<% redux_store("SharedReduxStore", props: @app_props_server_render, defer: true) %>

<%= react_component("ReduxSharedStoreApp", prerender: true, trace: true) %>
<%= react_component("ReduxSharedStoreApp", prerender: true, trace: true, id: "ReduxSharedStoreApp-react-component-0") %>
<hr/>
<h1>Second Hello World</h1>
<%= react_component("ReduxSharedStoreApp", prerender: true, trace: true) %>
<%= react_component("ReduxSharedStoreApp", prerender: true, trace: true, id: "ReduxSharedStoreApp-react-component-1") %>
<hr/>

<h1>React Rails Client Side Only Rendering, 2 components, same Redux store </h1>
Expand All @@ -30,10 +30,10 @@
<pre>
<%%= redux_store("SharedReduxStore", props: @app_props_server_render, defer: true) %>

<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-0") %>

Second Hello World
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true) %>
<%%= react_component("ReduxSharedStoreApp", prerender: false, trace: true, id: "ReduxSharedStoreApp-react-component-1") %>
</pre>
</li>
</ol>
2 changes: 1 addition & 1 deletion spec/dummy/app/views/pages/server_side_log_throw.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<%= render "header" %>

<%= react_component("HelloWorldWithLogAndThrow", props: @app_props_server_render, prerender: true, trace: true) %>
<%= react_component("HelloWorldWithLogAndThrow", props: @app_props_server_render, prerender: true, trace: true, id: "HelloWorldWithLogAndThrow-react-component-0") %>
<hr/>

<h1>React Rails Server Rendering</h1>
Expand Down
Loading

0 comments on commit ea3c7b3

Please sign in to comment.