From 496e26c6ef8d317cef4a7f5b2475bc1dad6c1eb6 Mon Sep 17 00:00:00 2001 From: Michael Baudino Date: Tue, 31 May 2016 09:13:33 +0400 Subject: [PATCH] Fix `dom_id` collisions by using a UUID rather than an autoincremented 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 #437 --- README.md | 2 +- app/helpers/react_on_rails_helper.rb | 11 +------ lib/react_on_rails/react_component/options.rb | 11 +++---- .../react_component/options_spec.rb | 32 +++++++------------ 4 files changed, 18 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index c3d6b49df5..6a68aac0b4 100644 --- a/README.md +++ b/README.md @@ -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. diff --git a/app/helpers/react_on_rails_helper.rb b/app/helpers/react_on_rails_helper.rb index 9ebb15dca7..a1efc4f3cb 100644 --- a/app/helpers/react_on_rails_helper.rb +++ b/app/helpers/react_on_rails_helper.rb @@ -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. @@ -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 diff --git a/lib/react_on_rails/react_component/options.rb b/lib/react_on_rails/react_component/options.rb index cf6e7397f7..54befc1012 100644 --- a/lib/react_on_rails/react_component/options.rb +++ b/lib/react_on_rails/react_component/options.rb @@ -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 @@ -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 @@ -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) diff --git a/spec/react_on_rails/react_component/options_spec.rb b/spec/react_on_rails/react_component/options_spec.rb index 3fea181cb3..f5311a46ce 100644 --- a/spec/react_on_rails/react_component/options_spec.rb +++ b/spec/react_on_rails/react_component/options_spec.rb @@ -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 @@ -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)