Merge pull request #113 from shakacode/ryanaip-data-tags

Use data attrs when rendering react_components instead of script tags

Author: Web-Go-To

app/assets/javascripts/react_on_rails.js

@@ -1,58 +1,6 @@
 (function() {
   this.ReactOnRails = {};
-
-  ReactOnRails.clientRenderReactComponent = function(options) {
-    var componentName = options.componentName;
-    var domId = options.domId;
-    var props = options.props;
-    var trace = options.trace;
-    var generatorFunction = options.generatorFunction;
-    var expectTurboLinks = options.expectTurboLinks;
-
-    var renderIfDomNodePresent = function() {
-      try {
-        var domNode = document.getElementById(domId);
-        if (domNode) {
-          var reactElement = createReactElement(componentName, props,
-            domId, trace, generatorFunction);
-          provideClientReact().render(reactElement, domNode);
-        }
-      }
-      catch (e) {
-        ReactOnRails.handleError({
-          e: e,
-          componentName: componentName,
-          serverSide: false,
-        });
-      }
-    };
-
-    var turbolinksInstalled = (typeof Turbolinks !== 'undefined');
-    if (!expectTurboLinks || (!turbolinksInstalled && expectTurboLinks)) {
-      if (expectTurboLinks) {
-        console.warn('WARNING: NO TurboLinks detected in JS, but it is in your Gemfile');
-      }
-
-      document.addEventListener('DOMContentLoaded', function(event) {
-        renderIfDomNodePresent();
-      });
-    } else {
-      function onPageChange(event) {
-        var removePageChangeListener = function() {
-          document.removeEventListener('page:change', onPageChange);
-          document.removeEventListener('page:before-unload', removePageChangeListener);
-          var domNode = document.getElementById(domId);
-          provideClientReact().unmountComponentAtNode(domNode);
-        };
-
-        document.addEventListener('page:before-unload', removePageChangeListener);
-
-        renderIfDomNodePresent();
-      }
-
-      document.addEventListener('page:change', onPageChange);
-    }
-  };
+  var turbolinksInstalled = (typeof Turbolinks !== 'undefined');
 
   ReactOnRails.serverRenderReactComponent = function(options) {
     var componentName = options.componentName;
@@ -65,7 +13,8 @@
     var consoleReplay = '';
 
     try {
-      var reactElement = createReactElement(componentName, props, domId, trace, generatorFunction);
+      var reactElement = createReactElement(componentName, props,
+        domId, trace, generatorFunction);
       htmlResult = provideServerReact().renderToString(reactElement);
     }
     catch (e) {
@@ -151,6 +100,56 @@
     return consoleReplay;
   };
 
+  function forEachComponent(fn) {
+    var els = document.getElementsByClassName('js-react-on-rails-component');
+    for (var i = 0; i < els.length; i++) {
+      fn(els[i]);
+    };
+  }
+
+  function pageLoaded() {
+    forEachComponent(render);
+  }
+
+  function pageUnloaded() {
+    forEachComponent(unmount);
+  }
+
+  function unmount(el) {
+    var domId = el.getAttribute('data-dom-id');
+    var domNode = document.getElementById(domId);
+    provideClientReact().unmountComponentAtNode(domNode);
+  }
+
+  function render(el) {
+    var componentName = el.getAttribute('data-component-name');
+    var domId = el.getAttribute('data-dom-id');
+    var props = JSON.parse(el.getAttribute('data-props'));
+    var trace = JSON.parse(el.getAttribute('data-trace'));
+    var generatorFunction = JSON.parse(el.getAttribute('data-generator-function'));
+    var expectTurboLinks = JSON.parse(el.getAttribute('data-expect-turbo-links'));
+
+    if (!turbolinksInstalled && expectTurboLinks) {
+      console.warn('WARNING: NO TurboLinks detected in JS, but it is in your Gemfile');
+    }
+
+    try {
+      var domNode = document.getElementById(domId);
+      if (domNode) {
+        var reactElement = createReactElement(componentName, props,
+          domId, trace, generatorFunction);
+        provideClientReact().render(reactElement, domNode);
+      }
+    }
+    catch (e) {
+      ReactOnRails.handleError({
+        e: e,
+        componentName: componentName,
+        serverSide: false,
+      });
+    }
+  };
+
   function createReactElement(componentName, props, domId, trace, generatorFunction) {
     if (trace) {
       console.log('RENDERED ' + componentName + ' to dom node with id: ' + domId);
@@ -178,4 +177,14 @@
 
     return ReactDOMServer;
   }
+
+  // Install listeners when running on the client.
+  if (typeof document !== 'undefined') {
+    if (!turbolinksInstalled) {
+      document.addEventListener('DOMContentLoaded', pageLoaded);
+    } else {
+      document.addEventListener('page:before-unload', pageUnloaded);
+      document.addEventListener('page:change', pageLoaded);
+    }
+  }
 }.call(this));

app/helpers/react_on_rails_helper.rb

@@ -19,7 +19,7 @@ module ReactOnRailsHelper
   #      global.MyReactComponentApp = MyReactComponentApp;
   #    See spec/dummy/client/app/startup/serverGlobals.jsx and
   #      spec/dummy/client/app/startup/ClientApp.jsx for examples of this
-  # props: Ruby Hash which contains the properties to pass to the react object
+  # props: Ruby Hash or JSON string which contains the properties to pass to the react object
   #
   #  options:
   #    generator_function: <true/false> default is false, set to true if you want to use a
@@ -51,27 +51,24 @@ def react_component(component_name, props = {}, 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.
     turbolinks_loaded = Object.const_defined?(:Turbolinks)
-    # NOTE: props might include closing script tag that might cause XSS
-    props_string = sanitized_props_string(props)
-    page_loaded_js = <<-JS
-(function() {
-  var props = #{props_string};
-  ReactOnRails.clientRenderReactComponent({
-    componentName: '#{react_component_name}',
-    domId: '#{dom_id}',
-    props: props,
-    trace: #{trace(options)},
-    generatorFunction: #{generator_function(options)},
-    expectTurboLinks: #{turbolinks_loaded}
-  });
-})();
-    JS
 
-    data_from_server_script_tag = javascript_tag(page_loaded_js)
+    component_specification_tag =
+      content_tag(:div,
+                  "",
+                  class: "js-react-on-rails-component",
+                  style: "display:none",
+                  data: {
+                    component_name: react_component_name,
+                    props: props,
+                    trace: trace(options),
+                    generator_function: generator_function(options),
+                    expect_turbolinks: turbolinks_loaded,
+                    dom_id: dom_id
+                  })
 
     # Create the HTML rendering part
     server_rendered_html, console_script =
-      server_rendered_react_component_html(options, props_string, react_component_name, dom_id)
+      server_rendered_react_component_html(options, props, react_component_name, dom_id)
 
     content_tag_options = options.except(:generator_function, :prerender, :trace,
                                          :replay_console, :id, :react_component_name,
@@ -84,7 +81,7 @@ def react_component(component_name, props = {}, options = {})
 
     # IMPORTANT: Ensure that we mark string as html_safe to avoid escaping.
     <<-HTML.html_safe
-#{data_from_server_script_tag}
+#{component_specification_tag}
 #{rendered_output}
 #{replay_console(options) ? console_script : ''}
     HTML
@@ -133,12 +130,15 @@ def next_react_component_index
 
   # Returns Array [0]: html, [1]: script to console log
   # NOTE, these are NOT html_safe!
-  def server_rendered_react_component_html(options, props_string, react_component_name, dom_id)
+  def server_rendered_react_component_html(options, props, react_component_name, dom_id)
     return ["", ""] unless prerender(options)
 
     # Make sure that we use up-to-date server-bundle
     ReactOnRails::ServerRenderingPool.reset_pool_if_server_bundle_was_modified
 
+    # Since this code is not inserted on a web page, we don't need to escape.
+    props_string = props.is_a?(String) ? props : props.to_json
+
     wrapper_js = <<-JS
 (function() {
   var props = #{props_string};
@@ -154,7 +154,11 @@ def server_rendered_react_component_html(options, props_string, react_component_
 
     ReactOnRails::ServerRenderingPool.server_render_js_with_console_logging(wrapper_js)
   rescue ExecJS::ProgramError => err
-    raise ReactOnRails::ServerRenderingPool::PrerenderError.new(react_component_name, props_string, err)
+    raise ReactOnRails::ServerRenderingPool::PrerenderError.new(
+      react_component_name,
+      sanitized_props_string(props), # Sanitize as this might be browser logged
+      err
+    )
   end
 
   def trace(options)

spec/dummy/spec/helpers/react_on_rails_helper_spec.rb

@@ -35,26 +35,45 @@
     end
   end
   describe "#react_component" do
-    subject { react_component("App") }
+    subject { react_component("App", props) }
+
+    let(:props) do
+      { name: "My Test Name" }
+    end
 
     let(:react_component_div) do
       "<div id=\"App-react-component-0\"></div>"
     end
 
+    let(:id) { "App-react-component-0" }
+
+    let(:react_definition_div) do
+      "<div class=\"js-react-on-rails-component\"
+            style=\"display:none\"
+            data-component-name=\"App\"
+            data-props=\"{&quot;name&quot;:&quot;My Test Name&quot;}\"
+            data-trace=\"false\"
+            data-generator-function=\"false\"
+            data-expect-turbolinks=\"true\"
+            data-dom-id=\"#{id}\"></div>".squish
+    end
+
     it { expect(self).to respond_to :react_component }
 
     it { is_expected.to be_an_instance_of ActiveSupport::SafeBuffer }
-    it { is_expected.to start_with "<script>" }
+    it { is_expected.to start_with "<div" }
     it { is_expected.to end_with "</div>\n\n" }
     it { is_expected.to include react_component_div }
+    it { is_expected.to include react_definition_div }
 
     context "with 'id' option" do
-      subject { react_component("App", {}, id: id) }
+      subject { react_component("App", props, id: id) }
 
       let(:id) { "shaka_div" }
 
       it { is_expected.to include id }
       it { is_expected.not_to include react_component_div }
+      it { is_expected.to include react_definition_div }
     end
   end
 

spec/dummy/spec/requests/console_logging_spec.rb

@@ -6,8 +6,8 @@
     html_nodes = Nokogiri::HTML(response.body)
 
     expected = <<-JS
-console.log.apply(console, ["[SERVER] RENDERED HelloWorldWithLogAndThrow to dom node with id: \
-HelloWorldWithLogAndThrow-react-component-0"]);
+console.log.apply(console, ["[SERVER] RENDERED HelloWorldWithLogAndThrow to dom node \
+with id: HelloWorldWithLogAndThrow-react-component-0"]);
 console.log.apply(console, ["[SERVER] console.log in HelloWorld"]);
 console.warn.apply(console, ["[SERVER] console.warn in HelloWorld"]);
 console.error.apply(console, ["[SERVER] console.error in HelloWorld"]);
@@ -18,7 +18,8 @@
 
     expected_lines = expected.split("\n")
 
-    script_node = html_nodes.css("script")[2]
+    script_node = html_nodes.css("script")[1]
+
     expected_lines.each do |line|
       expect(script_node.inner_text).to include(line)
     end