diff --git a/CHANGELOG.md b/CHANGELOG.md index ed131747b..2e9ec9006 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ Contributors: please follow the recommendations outlined at [keepachangelog.com] ## [Unreleased] *Please add entries here for your pull requests.* +## Improved +- Use `; }, 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 860b55082..0a17948e7 100644 --- a/node_package/src/clientStartup.js +++ b/node_package/src/clientStartup.js @@ -5,8 +5,7 @@ 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'; +const REACT_ON_RAILS_STORE_ATTRIBUTE = 'data-js-react-on-rails-store'; function findContext() { if (typeof window.ReactOnRails !== 'undefined') { @@ -42,21 +41,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(REACT_ON_RAILS_STORE_ATTRIBUTE); + 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, REACT_ON_RAILS_STORE_ATTRIBUTE, railsContext); } function turbolinksVersion5() { @@ -91,10 +97,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 +137,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 +152,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/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..f28d86581 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,61 @@ let(:id) { "App-react-component-0" } - let(:react_definition_div) do - %().squish + let(:react_definition_script) do + '" end - let(:react_definition_div_no_params) do - %().squish + let(:react_definition_script_no_params) do + '" + end + + context "with json string props" do + let(:json_props) do + "{\"hello\":\"world\",\"free\":\"of charge\",\"x\":\"\"}" + end + + let(:props_sanitized) do + '{"hello":"world","free":"of charge","x":"\\u003c/script\\u003e\\u003cscrip'\ + "t\\u003ealert('foo')\\u003c/script\\u003e\"}" + end + + subject { react_component("App", props: json_props) } + it { is_expected.to include props_sanitized } end 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 "\s*$} } it { is_expected.to include react_component_div } - it { is_expected.to include react_definition_div } + it { is_expected.to include react_definition_script } context "with 'id' option" do subject { react_component("App", props: 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 - - context "with skip_display_none option true" do - before { ReactOnRails.configuration.skip_display_none = true } - - let(:react_definition_div_skip_display_none_true) do - "
".squish + let(:react_definition_script) do + '" end - it { is_expected.to include react_definition_div_skip_display_none_true } - end - - context "with skip_display_none option false" do - before { ReactOnRails.configuration.skip_display_none = false } - - let(:react_definition_div_skip_display_none_false) do - "
".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 +124,18 @@ { name: "My Test Name" } end - let(:react_store_div) do - %().squish + let(:react_store_script) do + '" 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 "" } + it { is_expected.to include react_store_script } end describe "#server_render_js" do diff --git a/spec/dummy/spec/requests/console_logging_spec.rb b/spec/dummy/spec/requests/console_logging_spec.rb index 81d7232fd..444173b35 100644 --- a/spec/dummy/spec/requests/console_logging_spec.rb +++ b/spec/dummy/spec/requests/console_logging_spec.rb @@ -18,10 +18,10 @@ expected_lines = expected.split("\n") - script_node = html_nodes.css("script")[1] + script_node = html_nodes.css("script#consoleReplayLog") expected_lines.each do |line| - expect(script_node.inner_text).to include(line) + expect(script_node.text).to include(line) end end end diff --git a/spec/dummy/spec/requests/server_render_check_spec.rb b/spec/dummy/spec/requests/server_render_check_spec.rb index 57c533375..6564d1e98 100644 --- a/spec/dummy/spec/requests/server_render_check_spec.rb +++ b/spec/dummy/spec/requests/server_render_check_spec.rb @@ -63,14 +63,14 @@ mail = DummyMailer.hello_email expect(mail.subject).to match "mail" expect(mail.body).to match "Mr. Mailing Server Side Rendering" - expect(mail.body).to match "inMailer":true" + expect(mail.body).to match "\"inMailer\":true" end it "sets inMailer properly" do get client_side_hello_world_path html_nodes = Nokogiri::HTML(response.body) - expect(html_nodes.css("div#js-react-on-rails-context").attr("data-rails-context").value) - .to match('inMailer\":false') + expect(html_nodes.at_css("script#js-react-on-rails-context").content) + .to match("\"inMailer\":false") end end diff --git a/spec/react_on_rails/configuration_spec.rb b/spec/react_on_rails/configuration_spec.rb index 6169b5a17..050fef62f 100644 --- a/spec/react_on_rails/configuration_spec.rb +++ b/spec/react_on_rails/configuration_spec.rb @@ -40,18 +40,5 @@ module ReactOnRails expect(ReactOnRails.configuration.server_bundle_js_file).to eq("something.js") expect(ReactOnRails.configuration.prerender).to eq(true) end - - context "skip display: none" do - it "will default false" do - expect(ReactOnRails.configuration.skip_display_none).to eq(false) - end - - it "will be true if set to true" do - ReactOnRails.configure do |config| - config.skip_display_none = true - end - expect(ReactOnRails.configuration.skip_display_none).to eq(true) - end - end end end diff --git a/spec/react_on_rails/react_component/options_spec.rb b/spec/react_on_rails/react_component/options_spec.rb index f5311a46c..063717363 100644 --- a/spec/react_on_rails/react_component/options_spec.rb +++ b/spec/react_on_rails/react_component/options_spec.rb @@ -44,17 +44,6 @@ def the_attrs(name: "App", options: {}) expect(opts.props).to eq(props) end end - - context "as JSON" do - it "returns props" do - json_props = { a_prop: 2 }.to_json - attrs = the_attrs(options: { props: json_props }) - - opts = described_class.new(attrs) - - expect(opts.props).to eq(json_props) - end - end end describe "#name" do @@ -137,30 +126,6 @@ def the_attrs(name: "App", options: {}) end end - describe "#style" do - context "skipped display none" do - it "returns nil" do - ReactOnRails.configuration.skip_display_none = true - attrs = the_attrs - - opts = described_class.new(attrs) - - expect(opts.style).to eq nil - end - end - - context "not skipped display none" do - it "returns value" do - ReactOnRails.configuration.skip_display_none = false - attrs = the_attrs - - opts = described_class.new(attrs) - - expect(opts.style).to eq "display:none" - end - end - end - CONFIGURABLE_OPTIONS.each do |option| describe "##{option}" do context "with #{option} option" do