diff --git a/app/helpers/react_on_rails_helper.rb b/app/helpers/react_on_rails_helper.rb index 45d1bdf28..bbd4b38c3 100644 --- a/app/helpers/react_on_rails_helper.rb +++ b/app/helpers/react_on_rails_helper.rb @@ -103,13 +103,10 @@ def react_component(component_name, 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. - - component_specification_tag = - content_tag(:div, - "", - class: "js-react-on-rails-component", - style: options.style, - data: options.data) + component_specification_tag = content_tag(:script, + json_safe_and_pretty(options.data).html_safe, + type: "application/json", + class: "js-react-on-rails-component") # Create the HTML rendering part result = server_rendered_react_component_html(options.props, options.name, options.dom_id, @@ -174,7 +171,7 @@ def redux_store_hydration_data end def sanitized_props_string(props) - props.is_a?(String) ? json_escape(props) : props.to_json + props.is_a?(String) ? ERB::Util.json_escape(props) : props.to_json end # Helper method to take javascript expression and returns the output from evaluating it. @@ -225,30 +222,36 @@ def server_render_js(js_expression, options = {}) private + def json_safe_and_pretty(something) + if Rails.env.development? + ERB::Util.json_escape(JSON.pretty_generate(something.as_json)) + else + ERB::Util.json_escape(something.to_json) + end + end + # prepend the rails_context if not yet applied def prepend_render_rails_context(render_value) return render_value if @rendered_rails_context - data = { - rails_context: rails_context(server_side: false) - } + data = rails_context(server_side: false) @rendered_rails_context = true - rails_context_content = content_tag(:div, - "", - id: "js-react-on-rails-context", - style: ReactOnRails.configuration.skip_display_none ? nil : "display:none", - data: data) + rails_context_content = content_tag(:script, + json_safe_and_pretty(data).html_safe, + type: "application/json", + id: "js-react-on-rails-context") + "#{rails_context_content}\n#{render_value}".html_safe end def render_redux_store_data(redux_store_data) - result = content_tag(:div, - "", - class: "js-react-on-rails-store", - style: ReactOnRails.configuration.skip_display_none ? nil : "display:none", - data: redux_store_data) + result = content_tag(:script, + json_safe_and_pretty(redux_store_data[:props]).html_safe, + type: "application/json", + "data-js-react-on-rails-store" => redux_store_data[:store_name].html_safe) + prepend_render_rails_context(result) end diff --git a/lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.tt b/lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.tt index 30def168f..480b93b35 100644 --- a/lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.tt +++ b/lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.tt @@ -70,9 +70,6 @@ ReactOnRails.configure do |config| # MISCELLANEOUS OPTIONS ################################################################################ - # Default is false, enable if your content security policy doesn't include `style-src: 'unsafe-inline'` - config.skip_display_none = false - # The server render method - either ExecJS or NodeJS <%- if options.node? -%> config.server_render_method = "NodeJS" diff --git a/lib/react_on_rails/configuration.rb b/lib/react_on_rails/configuration.rb index cfe827164..b141f2fc4 100644 --- a/lib/react_on_rails/configuration.rb +++ b/lib/react_on_rails/configuration.rb @@ -76,7 +76,6 @@ def self.configuration development_mode: Rails.env.development?, server_renderer_pool_size: 1, server_renderer_timeout: 20, - skip_display_none: false, webpack_generated_files: [], rendering_extension: nil, server_render_method: "", @@ -92,7 +91,7 @@ class Configuration :trace, :development_mode, :logging_on_server, :server_renderer_pool_size, :server_renderer_timeout, :raise_on_prerender_error, - :skip_display_none, :generated_assets_dirs, :generated_assets_dir, + :generated_assets_dirs, :generated_assets_dir, :webpack_generated_files, :rendering_extension, :npm_build_test_command, :npm_build_production_command, :i18n_dir, @@ -102,7 +101,7 @@ def initialize(server_bundle_js_file: nil, prerender: nil, replay_console: nil, trace: nil, development_mode: nil, logging_on_server: nil, server_renderer_pool_size: nil, server_renderer_timeout: nil, raise_on_prerender_error: nil, - skip_display_none: nil, generated_assets_dirs: nil, + generated_assets_dirs: nil, generated_assets_dir: nil, webpack_generated_files: nil, rendering_extension: nil, npm_build_test_command: nil, npm_build_production_command: nil, @@ -125,7 +124,6 @@ def initialize(server_bundle_js_file: nil, prerender: nil, replay_console: nil, end self.trace = trace.nil? ? Rails.env.development? : trace self.raise_on_prerender_error = raise_on_prerender_error - self.skip_display_none = skip_display_none # Server rendering: self.server_renderer_pool_size = self.development_mode ? 1 : server_renderer_pool_size diff --git a/lib/react_on_rails/react_component/options.rb b/lib/react_on_rails/react_component/options.rb index 711de82f7..8ee597886 100644 --- a/lib/react_on_rails/react_component/options.rb +++ b/lib/react_on_rails/react_component/options.rb @@ -55,7 +55,6 @@ def data end def style - return nil if ReactOnRails.configuration.skip_display_none HIDDEN end diff --git a/node_package/src/RenderUtils.js b/node_package/src/RenderUtils.js index 14fbe7598..70c13b7a2 100644 --- a/node_package/src/RenderUtils.js +++ b/node_package/src/RenderUtils.js @@ -1,10 +1,10 @@ export default { - wrapInScriptTags(scriptBody) { + wrapInScriptTags(scriptId, scriptBody) { if (!scriptBody) { return ''; } - return `\n`; }, diff --git a/node_package/src/buildConsoleReplay.js b/node_package/src/buildConsoleReplay.js index 056a58a0a..7a8005651 100644 --- a/node_package/src/buildConsoleReplay.js +++ b/node_package/src/buildConsoleReplay.js @@ -28,5 +28,5 @@ export function consoleReplay() { } export default function buildConsoleReplay() { - return RenderUtils.wrapInScriptTags(consoleReplay()); + return RenderUtils.wrapInScriptTags('consoleReplayLog', consoleReplay()); } diff --git a/node_package/src/clientStartup.js b/node_package/src/clientStartup.js index f3eb4848e..a573564f4 100644 --- a/node_package/src/clientStartup.js +++ b/node_package/src/clientStartup.js @@ -5,9 +5,6 @@ import ReactDOM from 'react-dom'; import createReactElement from './createReactElement'; import isRouterResult from './isCreateReactElementResultNonReactComponent'; -const REACT_ON_RAILS_COMPONENT_CLASS_NAME = 'js-react-on-rails-component'; -const REACT_ON_RAILS_STORE_CLASS_NAME = 'js-react-on-rails-store'; - function findContext() { if (typeof window.ReactOnRails !== 'undefined') { return window; @@ -42,21 +39,28 @@ function forEach(fn, className, railsContext) { } } +function forEachByAttribute(fn, attributeName, railsContext) { + const els = document.querySelectorAll(`[${attributeName}]`); + for (let i = 0; i < els.length; i += 1) { + fn(els[i], railsContext); + } +} + function forEachComponent(fn, railsContext) { - forEach(fn, REACT_ON_RAILS_COMPONENT_CLASS_NAME, railsContext); + forEach(fn, 'js-react-on-rails-component', railsContext); } function initializeStore(el, railsContext) { const context = findContext(); - const name = el.getAttribute('data-store-name'); - const props = JSON.parse(el.getAttribute('data-props')); + const name = el.getAttribute('data-js-react-on-rails-store'); + const props = JSON.parse(el.textContent); const storeGenerator = context.ReactOnRails.getStoreGenerator(name); const store = storeGenerator(props, railsContext); context.ReactOnRails.setStore(name, store); } function forEachStore(railsContext) { - forEach(initializeStore, REACT_ON_RAILS_STORE_CLASS_NAME, railsContext); + forEachByAttribute(initializeStore, 'data-js-react-on-rails-store', railsContext); } function turbolinksVersion5() { @@ -91,10 +95,11 @@ DELEGATING TO RENDERER ${name} for dom node with id: ${domNodeId} with props, ra */ function render(el, railsContext) { const context = findContext(); - const name = el.getAttribute('data-component-name'); - const domNodeId = el.getAttribute('data-dom-id'); - const props = JSON.parse(el.getAttribute('data-props')); - const trace = JSON.parse(el.getAttribute('data-trace')); + const jsonEl = JSON.parse(el.textContent); + const name = jsonEl.component_name; + const domNodeId = jsonEl.dom_id; + const props = jsonEl.props; + const trace = jsonEl.trace; try { const domNode = document.getElementById(domNodeId); @@ -130,7 +135,7 @@ You should return a React.Component always for the client side entry point.`); function parseRailsContext() { const el = document.getElementById('js-react-on-rails-context'); if (el) { - return JSON.parse(el.getAttribute('data-rails-context')); + return JSON.parse(el.textContent); } return null; @@ -145,7 +150,8 @@ export function reactOnRailsPageLoaded() { } function unmount(el) { - const domNodeId = el.getAttribute('data-dom-id'); + const elData = JSON.parse(el.textContent); + const domNodeId = elData.dom_id; const domNode = document.getElementById(domNodeId); ReactDOM.unmountComponentAtNode(domNode); } diff --git a/node_package/tests/buildConsoleReplay.test.js b/node_package/tests/buildConsoleReplay.test.js index 374ff6f8b..5d305405a 100644 --- a/node_package/tests/buildConsoleReplay.test.js +++ b/node_package/tests/buildConsoleReplay.test.js @@ -85,7 +85,7 @@ test('buildConsoleReplay wraps console replay in a script tag', (assert) => { const actual = buildConsoleReplay(); const expected = ` -`; diff --git a/spec/dummy/app/controllers/pages_controller.rb b/spec/dummy/app/controllers/pages_controller.rb index f1ad082cb..dfcec54c0 100644 --- a/spec/dummy/app/controllers/pages_controller.rb +++ b/spec/dummy/app/controllers/pages_controller.rb @@ -26,23 +26,24 @@ def initialize_shared_store end def data + xss_payload = { "" => '' } # This is the props used by the React component. @app_props_server_render = { helloWorldData: { name: "Mr. Server Side Rendering" - } + }.merge(xss_payload) } @app_props_hello = { helloWorldData: { name: "Mrs. Client Side Rendering" - } + }.merge(xss_payload) } @app_props_hello_again = { helloWorldData: { name: "Mrs. Client Side Hello Again" - } + }.merge(xss_payload) } end end diff --git a/spec/dummy/app/views/pages/server_side_hello_world_with_options.html.erb b/spec/dummy/app/views/pages/server_side_hello_world_with_options.html.erb index 00d8463bd..b1fcdc506 100644 --- a/spec/dummy/app/views/pages/server_side_hello_world_with_options.html.erb +++ b/spec/dummy/app/views/pages/server_side_hello_world_with_options.html.erb @@ -1,7 +1,7 @@ <%= render "header" %> <%= react_component("HelloWorld", - props: @app_props_server_render.to_json, + props: @app_props_server_render, prerender: true, trace: true, id: "my-hello-world-id", @@ -27,7 +27,7 @@
<%%= react_component("HelloWorld", - props: @app_props_server_render.to_json, + props: @app_props_server_render, prerender: true, trace: true, id: "my-hello-world-id", diff --git a/spec/dummy/client/webpack.client.rails.hot.config.js b/spec/dummy/client/webpack.client.rails.hot.config.js index babaec68b..ee185e2bb 100644 --- a/spec/dummy/client/webpack.client.rails.hot.config.js +++ b/spec/dummy/client/webpack.client.rails.hot.config.js @@ -25,7 +25,7 @@ config.output = { config.module.rules.push( { test: /\.jsx?$/, - loader: 'babel', + loader: 'babel-loader', exclude: /node_modules/, query: { plugins: [ diff --git a/spec/dummy/config/initializers/react_on_rails.rb b/spec/dummy/config/initializers/react_on_rails.rb index f237bef7a..540c192b8 100644 --- a/spec/dummy/config/initializers/react_on_rails.rb +++ b/spec/dummy/config/initializers/react_on_rails.rb @@ -81,8 +81,6 @@ def self.custom_context(view_context) ################################################################################ # MISCELLANEOUS OPTIONS ################################################################################ - # Default is false, enable if your content security policy doesn't include `style-src: 'unsafe-inline'` - config.skip_display_none = false # This allows you to add additional values to the Rails Context. Implement one static method # called `custom_context(view_context)` and return a Hash. diff --git a/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb b/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb index b212c2d46..de97c706b 100644 --- a/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb +++ b/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb @@ -59,76 +59,44 @@ let(:id) { "App-react-component-0" } - let(:react_definition_div) do - %().squish + # rubocop:disable Metrics/LineLength + let(:react_definition_script) do + %().squish end - let(:react_definition_div_no_params) do - %().squish + let(:react_definition_script_no_params) do + %().squish end + # rubocop:enable Metrics/LineLength describe "API with component name only" do subject { react_component("App") } it { is_expected.to be_an_instance_of ActiveSupport::SafeBuffer } it { is_expected.to include react_component_div } - it { is_expected.to include react_definition_div_no_params } + it { is_expected.to include react_definition_script_no_params } 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 "\s*$} } + it { is_expected.to start_with ").squish end - it { is_expected.to include react_definition_div_skip_display_none_false } + it { is_expected.to include id } + it { is_expected.not_to include react_component_div } + it { is_expected.to include react_definition_script } end end @@ -139,36 +107,16 @@ { name: "My Test Name" } end - let(:react_store_div) do - %().squish + let(:react_store_script) do + %().squish end it { expect(self).to respond_to :redux_store } it { is_expected.to be_an_instance_of ActiveSupport::SafeBuffer } - it { is_expected.to start_with "" } - it { is_expected.to include react_store_div } - - context "with skip_display_none option true" do - before { ReactOnRails.configuration.skip_display_none = true } - - let(:react_store_definition_div_skip_display_none_true) do - %().squish - end - - it { is_expected.to include react_store_definition_div_skip_display_none_true } - end - - context "with skip_display_none option false" do - before { ReactOnRails.configuration.skip_display_none = false } - it { is_expected.to include react_store_div } - end + it { is_expected.to start_with "