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).

Solves shakacode#437
  • Loading branch information
michaelbaudino committed May 31, 2016
1 parent c7b9209 commit 496e26c
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 38 deletions.
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
32 changes: 12 additions & 20 deletions spec/react_on_rails/react_component/options_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@
raise_on_prerender_error
).freeze

def the_attrs(name: "App", index: 1, options: {})
def the_attrs(name: "App", options: {})
{
name: name,
index: index,
options: options
}
end
Expand Down Expand Up @@ -69,32 +68,25 @@ def the_attrs(name: "App", index: 1, options: {})
end
end

describe "#index" do
it "returns index" do
index = 2
attrs = the_attrs(index: index)

opts = described_class.new(attrs)

expect(opts.index).to eq index
end
end

describe "#dom_id" do
context "without id option" do
it "returns dom_id" do
index = 2
name = "some_app"
attrs = the_attrs(name: name, index: index)

it "returns a unique identifier" do
attrs = the_attrs(name: "some_app")
opts = described_class.new(attrs)

expect(opts.dom_id).to eq "some_app-react-component-2"
expect(SecureRandom).to receive(:uuid).and_return("123456789")
expect(opts.dom_id).to eq "some_app-react-component-123456789"
end

it "is memoized" do
opts = described_class.new(the_attrs)

expect(opts.dom_id).to eq opts.dom_id
end
end

context "with id option" do
it "returns dom_id" do
it "returns given id" do
options = { id: "im-an-id" }
attrs = the_attrs(options: options)

Expand Down

0 comments on commit 496e26c

Please sign in to comment.