From 342c6ee41e644346f1468e20b92209d5bccca600 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Sat, 30 Jul 2022 00:51:25 -0400 Subject: [PATCH 1/9] Add some failing tests for `StimulusReflex::Fragment` when used with a whole HTML document --- test/fragment_test.rb | 68 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 test/fragment_test.rb diff --git a/test/fragment_test.rb b/test/fragment_test.rb new file mode 100644 index 00000000..53a8c90c --- /dev/null +++ b/test/fragment_test.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require_relative "test_helper" + +class StimulusReflex::FragmentTest < ActiveSupport::TestCase + test "should extract part of HTML" do + raw_html = <<-HTML +
+

Home#index

+
+ HTML + + fragment = StimulusReflex::Fragment.new(raw_html) + + assert_equal fragment.match("#title").to_html.squish, 'Home#index' + end + + test "should extract top-level element of fragement" do + raw_html = <<-HTML +
+

Home#index

+
+ HTML + + fragment = StimulusReflex::Fragment.new(raw_html) + + assert_equal fragment.to_html.squish, raw_html.squish + assert_equal fragment.match("#container").to_html.squish, '

Home#index

' + end + + test "should extract body of a fragment" do + raw_html = <<-HTML + +

Home#index

+

Find me in app/views/home/index.html.erb

+ + HTML + + fragment = StimulusReflex::Fragment.new(raw_html) + + assert_equal fragment.to_html.squish, raw_html.squish + end + + test "should extract whole HTML document" do + raw_html = <<-HTML + + + + StimulusReflex Test + + + + + + + + +

Home#index

+

Find me in app/views/home/index.html.erb

+ + + HTML + + fragment = StimulusReflex::Fragment.new(raw_html) + + assert_equal fragment.to_html.squish, raw_html.squish + end +end From 8c1086864992874c88fe202c3581bbcef1514fa1 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Sat, 30 Jul 2022 00:56:14 -0400 Subject: [PATCH 2/9] Add extract assertion for extracting the body out of a whole document --- test/fragment_test.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/test/fragment_test.rb b/test/fragment_test.rb index 53a8c90c..fd9f95d8 100644 --- a/test/fragment_test.rb +++ b/test/fragment_test.rb @@ -42,6 +42,13 @@ class StimulusReflex::FragmentTest < ActiveSupport::TestCase end test "should extract whole HTML document" do + raw_body = <<-BODY + +

Home#index

+

Find me in app/views/home/index.html.erb

+ + BODY + raw_html = <<-HTML @@ -54,15 +61,14 @@ class StimulusReflex::FragmentTest < ActiveSupport::TestCase - -

Home#index

-

Find me in app/views/home/index.html.erb

- + #{raw_body} HTML fragment = StimulusReflex::Fragment.new(raw_html) + assert_equal fragment.match("body").to_html.squish, raw_body.squish + assert_equal fragment.match("#body").to_html.squish, raw_body.squish assert_equal fragment.to_html.squish, raw_html.squish end end From baec564707a49a1dd52a6de7f429e9c20bd90af9 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Sat, 30 Jul 2022 00:51:25 -0400 Subject: [PATCH 3/9] Fix `StimulusReflex::Fragment` class in combination with page morphs --- lib/stimulus_reflex.rb | 4 +- .../broadcasters/broadcaster.rb | 3 - .../broadcasters/page_broadcaster.rb | 2 +- .../broadcasters/selector_broadcaster.rb | 2 +- lib/stimulus_reflex/fragment.rb | 30 ---- lib/stimulus_reflex/html/document.rb | 9 ++ lib/stimulus_reflex/html/fragment.rb | 11 ++ lib/stimulus_reflex/html/part.rb | 64 +++++++++ test/html/document_test.rb | 131 ++++++++++++++++++ test/html/fragment_test.rb | 119 ++++++++++++++++ 10 files changed, 339 insertions(+), 36 deletions(-) delete mode 100644 lib/stimulus_reflex/fragment.rb create mode 100644 lib/stimulus_reflex/html/document.rb create mode 100644 lib/stimulus_reflex/html/fragment.rb create mode 100644 lib/stimulus_reflex/html/part.rb create mode 100644 test/html/document_test.rb create mode 100644 test/html/fragment_test.rb diff --git a/lib/stimulus_reflex.rb b/lib/stimulus_reflex.rb index d94014cf..0fbd3d47 100644 --- a/lib/stimulus_reflex.rb +++ b/lib/stimulus_reflex.rb @@ -13,7 +13,9 @@ require "stimulus_reflex/concern_enhancer" require "stimulus_reflex/configuration" require "stimulus_reflex/callbacks" -require "stimulus_reflex/fragment" +require "stimulus_reflex/html/part" +require "stimulus_reflex/html/fragment" +require "stimulus_reflex/html/document" require "stimulus_reflex/request_parameters" require "stimulus_reflex/reflex" require "stimulus_reflex/reflex_data" diff --git a/lib/stimulus_reflex/broadcasters/broadcaster.rb b/lib/stimulus_reflex/broadcasters/broadcaster.rb index 35176d0f..1e4f1c7b 100644 --- a/lib/stimulus_reflex/broadcasters/broadcaster.rb +++ b/lib/stimulus_reflex/broadcasters/broadcaster.rb @@ -5,9 +5,6 @@ class Broadcaster attr_reader :reflex, :cable_ready, :logger, :operations delegate :permanent_attribute_name, :payload, to: :reflex - DEFAULT_HTML_WITHOUT_FORMAT = Nokogiri::XML::Node::SaveOptions::DEFAULT_HTML & - ~Nokogiri::XML::Node::SaveOptions::FORMAT - def initialize(reflex) @reflex = reflex @logger = Rails.logger if defined?(Rails.logger) diff --git a/lib/stimulus_reflex/broadcasters/page_broadcaster.rb b/lib/stimulus_reflex/broadcasters/page_broadcaster.rb index 473c5ead..2d1341aa 100644 --- a/lib/stimulus_reflex/broadcasters/page_broadcaster.rb +++ b/lib/stimulus_reflex/broadcasters/page_broadcaster.rb @@ -4,7 +4,7 @@ module StimulusReflex class PageBroadcaster < Broadcaster def broadcast(selectors, data) reflex.controller.process reflex.params[:action] - fragment = StimulusReflex::Fragment.new(reflex.controller.response.body) + fragment = StimulusReflex::HTML::Document.new(reflex.controller.response.body) return if fragment.empty? diff --git a/lib/stimulus_reflex/broadcasters/selector_broadcaster.rb b/lib/stimulus_reflex/broadcasters/selector_broadcaster.rb index 8bcb8a6c..a15ecb82 100644 --- a/lib/stimulus_reflex/broadcasters/selector_broadcaster.rb +++ b/lib/stimulus_reflex/broadcasters/selector_broadcaster.rb @@ -7,7 +7,7 @@ def broadcast(_, data = {}) selectors, html = morph updates = create_update_collection(selectors, html) updates.each do |update| - fragment = StimulusReflex::Fragment.new(update.html) + fragment = StimulusReflex::HTML::Fragment.new(update.html) match = fragment.match(update.selector) if match.present? operations << [update.selector, StimulusReflex.config.morph_operation] diff --git a/lib/stimulus_reflex/fragment.rb b/lib/stimulus_reflex/fragment.rb deleted file mode 100644 index bf4c606e..00000000 --- a/lib/stimulus_reflex/fragment.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -module StimulusReflex - class Fragment - delegate :to_html, to: :@fragment - - def initialize(html) - @fragment = Nokogiri::HTML.fragment(html.to_s) - @matches = { - "body" => Match.new(@fragment) - } - end - - def empty? - @fragment.content.empty? - end - - def match(selector) - @matches[selector] ||= Match.new(@fragment.at_css(selector)) - end - - Match = Struct.new(:element) do - delegate :present?, to: :element - - def to_html - element&.inner_html(save_with: Broadcaster::DEFAULT_HTML_WITHOUT_FORMAT) - end - end - end -end diff --git a/lib/stimulus_reflex/html/document.rb b/lib/stimulus_reflex/html/document.rb new file mode 100644 index 00000000..d8a8bec7 --- /dev/null +++ b/lib/stimulus_reflex/html/document.rb @@ -0,0 +1,9 @@ +module StimulusReflex + module HTML + class Document < Part + def nokogiri_parser(html) + Nokogiri::HTML5::Document.parse(html) + end + end + end +end diff --git a/lib/stimulus_reflex/html/fragment.rb b/lib/stimulus_reflex/html/fragment.rb new file mode 100644 index 00000000..607e0132 --- /dev/null +++ b/lib/stimulus_reflex/html/fragment.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module StimulusReflex + module HTML + class Fragment < Part + def nokogiri_parser(html) + Nokogiri::HTML.fragment(html) + end + end + end +end diff --git a/lib/stimulus_reflex/html/part.rb b/lib/stimulus_reflex/html/part.rb new file mode 100644 index 00000000..247481f1 --- /dev/null +++ b/lib/stimulus_reflex/html/part.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +module StimulusReflex + module HTML + class Part + DEFAULT_HTML_WITHOUT_FORMAT = Nokogiri::XML::Node::SaveOptions::DEFAULT_HTML & ~Nokogiri::XML::Node::SaveOptions::FORMAT + + delegate :element, to: :@fragment + + def to_html + @fragment.to_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) + end + + def outer_html + @fragment.to_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) + end + + def inner_html + @fragment.inner_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) + end + + def nokogiri_parser(html) + raise "not implemented" + end + + def initialize(html) + @fragment = nokogiri_parser(html.to_s) + @matches = { + "body" => Match.new(@fragment.at_css("body")) + } + end + + def empty? + @fragment.content.empty? + end + + def match(selector) + @matches[selector] ||= Match.new(@fragment.at_css(selector)) + end + + Match = Struct.new(:element) do + delegate :present?, to: :element + + def outer_html + element&.to_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) + end + + # TODO: uncomment if the other to_html method is renamed + # def to_html + # element&.to_html + # end + + #  TODO: this method should be renamed to inner_html + def to_html + element&.inner_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) + end + + def inner_html + element&.inner_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) + end + end + end + end +end diff --git a/test/html/document_test.rb b/test/html/document_test.rb new file mode 100644 index 00000000..1bfadffd --- /dev/null +++ b/test/html/document_test.rb @@ -0,0 +1,131 @@ +# frozen_string_literal: true + +require_relative "../test_helper" + +class StimulusReflex::HTML::DocumentTest < ActiveSupport::TestCase + test "should handle nil" do + document = StimulusReflex::HTML::Document.new(nil) + + assert_equal "", document.to_html + assert_equal "", document.outer_html + assert_equal "", document.inner_html + end + + test "should handle empty string" do + document = StimulusReflex::HTML::Document.new("") + + assert_equal "", document.to_html + assert_equal "", document.outer_html + assert_equal "", document.inner_html + end + + test "should accept fragement and build whole document of HTML" do + raw_html = <<-HTML +
+

Home#index

+
+ HTML + + document = StimulusReflex::HTML::Document.new(raw_html) + + inner_title = "Home#index" + outer_title = "

Home#index

" + inner_container = outer_title + outer_container = "
#{inner_container}
" + inner_body = outer_container + outer_body = "#{inner_body} " + whole_document = "#{outer_body}" + + # TODO: this should be have like outer_html + assert_equal inner_body, document.match("body").to_html.squish + assert_equal inner_container, document.match("#container").to_html.squish + assert_equal inner_title, document.match("#title").to_html.squish + + assert_equal whole_document, document.to_html.squish + assert_equal whole_document, document.inner_html.squish + assert_equal whole_document, document.outer_html.squish + + assert_equal inner_body, document.match("body").inner_html.squish + assert_equal outer_body, document.match("body").outer_html.squish + + assert_equal inner_container, document.match("#container").inner_html.squish + assert_equal outer_container, document.match("#container").outer_html.squish + + assert_equal outer_title, document.match("#title").outer_html.squish + assert_equal inner_title, document.match("#title").inner_html.squish + end + + test "should extract body of a document" do + raw_html = <<-HTML + +

Home#index

+

Find me in app/views/home/index.html.erb

+ + HTML + + document = StimulusReflex::HTML::Document.new(raw_html) + + inner_body = "

Home#index

Find me in app/views/home/index.html.erb

" + outer_body = " #{inner_body} " + whole_document = "#{outer_body}" + + assert_equal whole_document, document.to_html.squish + assert_equal whole_document, document.outer_html.squish + assert_equal whole_document, document.inner_html.squish + + assert_equal outer_body, document.match("body").outer_html.squish + assert_equal outer_body, document.match("#body").outer_html.squish + + assert_equal inner_body, document.match("#body").inner_html.squish + assert_equal inner_body, document.match("body").inner_html.squish + + # TODO: this should behave like #outer_html + assert_equal inner_body, document.match("body").to_html.squish + assert_equal inner_body, document.match("#body").to_html.squish + end + + test "should extract whole HTML document" do + raw_body = <<-BODY + +

Home#index

+

Find me in app/views/home/index.html.erb

+ + BODY + + raw_html = <<-HTML + + + + StimulusReflex Test + + + + + + + + #{raw_body} + + HTML + + inner_body = "

Home#index

Find me in app/views/home/index.html.erb

" + outer_body = " #{inner_body} " + expected_html = " StimulusReflex Test #{outer_body}" + + document = StimulusReflex::HTML::Document.new(raw_html) + + assert_equal expected_html, document.to_html.squish + assert_equal expected_html, document.outer_html.squish + assert_equal expected_html, document.inner_html.squish + + assert_equal outer_body, document.match("body").outer_html.squish + assert_equal outer_body, document.match("#body").outer_html.squish + + assert_equal inner_body, document.match("body").inner_html.squish + assert_equal inner_body, document.match("#body").inner_html.squish + + # TODO: change this to outer_body + assert_equal inner_body, document.match("body").to_html.squish + assert_equal inner_body, document.match("#body").to_html.squish + end +end diff --git a/test/html/fragment_test.rb b/test/html/fragment_test.rb new file mode 100644 index 00000000..a007c97b --- /dev/null +++ b/test/html/fragment_test.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +require_relative "../test_helper" + +class StimulusReflex::HTML::FragmentTest < ActiveSupport::TestCase + test "should handle nil" do + fragment = StimulusReflex::HTML::Fragment.new(nil) + + assert_equal "", fragment.to_html + assert_equal "", fragment.outer_html + assert_equal "", fragment.inner_html + end + + test "should handle empty string" do + fragment = StimulusReflex::HTML::Fragment.new("") + + assert_equal "", fragment.to_html + assert_equal "", fragment.outer_html + assert_equal "", fragment.inner_html + end + + test "should extract a fragment of the HTML" do + raw_html = <<-HTML +
+

Home#index

+
+ HTML + + fragment = StimulusReflex::HTML::Fragment.new(raw_html) + + inner_title = "Home#index" + outer_title = "

#{inner_title}

" + + assert_equal raw_html.squish, fragment.to_html.squish + assert_equal raw_html.squish, fragment.inner_html.squish + assert_equal raw_html.squish, fragment.outer_html.squish + + refute fragment.match("body").present? + assert_nil fragment.match("body").to_html + + assert_equal outer_title, fragment.match("#container").to_html.squish + assert_equal inner_title, fragment.match("#title").to_html.squish + end + + test "should extract body of a fragment" do + raw_body = <<-HTML + +

Home#index

+

Find me in app/views/home/index.html.erb

+ + HTML + + fragment = StimulusReflex::HTML::Fragment.new(raw_body) + + inner_body = "

Home#index

Find me in app/views/home/index.html.erb

" + outer_body = " #{inner_body} " + + assert_equal outer_body, fragment.to_html.squish + assert_equal outer_body, fragment.inner_html.squish + assert_equal outer_body, fragment.outer_html.squish + + # TODO: change to outer_body + assert_equal inner_body, fragment.match("body").to_html.squish + assert_equal outer_body, fragment.match("body").outer_html.squish + assert_equal inner_body, fragment.match("body").inner_html.squish + + # Nokogiri fragment's ignore the body, head and html tag if passed into them. That's why this is expected to be nil + # if the body, head or HTML tags matter you should use `StimulusReflex::HTML::Document` instead + refute fragment.match("#body").present? + assert_nil fragment.match("#body").outer_html + assert_nil fragment.match("#body").to_html + assert_nil fragment.match("#body").inner_html + end + + test "should extract whole HTML document" do + raw_body = <<-BODY + +

Home#index

+

Find me in app/views/home/index.html.erb

+ + BODY + + raw_html = <<-HTML + + + + StimulusReflex Test + + + + + + + + #{raw_body} + + HTML + + fragment = StimulusReflex::HTML::Fragment.new(raw_html) + + # Nokogiri fragments strip out the body, head and html tag and just stuff every other tag into the body the as well, even if they don't belong there + # This is super unexpected, but that's the way Nokogiri fragements work + inner_body = "StimulusReflex Test

Home#index

Find me in app/views/home/index.html.erb

" + outer_body = " #{inner_body} " + + assert_equal outer_body, fragment.to_html.squish + assert_equal outer_body, fragment.outer_html.squish + assert_equal outer_body, fragment.inner_html.squish + + assert_equal outer_body, fragment.match("body").outer_html.squish + assert_equal inner_body, fragment.match("body").inner_html.squish + + # TODO: change this to outer_html + assert_equal inner_body, fragment.match("body").to_html.squish + + refute fragment.match("#body").present? + assert_nil fragment.match("#body").to_html + end +end From cf28894479a385eec66bea98484158423a5cc8d5 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Thu, 11 Aug 2022 08:45:03 +0200 Subject: [PATCH 4/9] change `to_html` to default to `outer_html` instead of `inner_html` --- .../broadcasters/page_broadcaster.rb | 2 +- .../broadcasters/selector_broadcaster.rb | 4 ++-- lib/stimulus_reflex/html/part.rb | 8 +------- test/html/document_test.rb | 17 +++++++---------- test/html/fragment_test.rb | 11 ++++------- 5 files changed, 15 insertions(+), 27 deletions(-) diff --git a/lib/stimulus_reflex/broadcasters/page_broadcaster.rb b/lib/stimulus_reflex/broadcasters/page_broadcaster.rb index 2d1341aa..bc9b51b6 100644 --- a/lib/stimulus_reflex/broadcasters/page_broadcaster.rb +++ b/lib/stimulus_reflex/broadcasters/page_broadcaster.rb @@ -11,7 +11,7 @@ def broadcast(selectors, data) selectors = selectors.select { |s| fragment.match(s).present? } selectors.each do |selector| operations << [selector, StimulusReflex.config.morph_operation] - html = fragment.match(selector).to_html + html = fragment.match(selector).inner_html cable_ready.send StimulusReflex.config.morph_operation, { selector: selector, html: html, diff --git a/lib/stimulus_reflex/broadcasters/selector_broadcaster.rb b/lib/stimulus_reflex/broadcasters/selector_broadcaster.rb index a15ecb82..bcef0089 100644 --- a/lib/stimulus_reflex/broadcasters/selector_broadcaster.rb +++ b/lib/stimulus_reflex/broadcasters/selector_broadcaster.rb @@ -13,7 +13,7 @@ def broadcast(_, data = {}) operations << [update.selector, StimulusReflex.config.morph_operation] cable_ready.send StimulusReflex.config.morph_operation, { selector: update.selector, - html: match.to_html, + html: match.inner_html, payload: payload, children_only: true, permanent_attribute_name: permanent_attribute_name, @@ -23,7 +23,7 @@ def broadcast(_, data = {}) operations << [update.selector, StimulusReflex.config.replace_operation] cable_ready.send StimulusReflex.config.replace_operation, { selector: update.selector, - html: fragment.to_html, + html: fragment.inner_html, payload: payload, stimulus_reflex: data.merge(morph: to_sym) } diff --git a/lib/stimulus_reflex/html/part.rb b/lib/stimulus_reflex/html/part.rb index 247481f1..a31855bc 100644 --- a/lib/stimulus_reflex/html/part.rb +++ b/lib/stimulus_reflex/html/part.rb @@ -45,14 +45,8 @@ def outer_html element&.to_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) end - # TODO: uncomment if the other to_html method is renamed - # def to_html - # element&.to_html - # end - - #  TODO: this method should be renamed to inner_html def to_html - element&.inner_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) + element&.to_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) end def inner_html diff --git a/test/html/document_test.rb b/test/html/document_test.rb index 1bfadffd..74a9bad9 100644 --- a/test/html/document_test.rb +++ b/test/html/document_test.rb @@ -36,10 +36,9 @@ class StimulusReflex::HTML::DocumentTest < ActiveSupport::TestCase outer_body = "#{inner_body} " whole_document = "#{outer_body}" - # TODO: this should be have like outer_html - assert_equal inner_body, document.match("body").to_html.squish - assert_equal inner_container, document.match("#container").to_html.squish - assert_equal inner_title, document.match("#title").to_html.squish + assert_equal outer_body, document.match("body").to_html.squish + assert_equal outer_container, document.match("#container").to_html.squish + assert_equal outer_title, document.match("#title").to_html.squish assert_equal whole_document, document.to_html.squish assert_equal whole_document, document.inner_html.squish @@ -79,9 +78,8 @@ class StimulusReflex::HTML::DocumentTest < ActiveSupport::TestCase assert_equal inner_body, document.match("#body").inner_html.squish assert_equal inner_body, document.match("body").inner_html.squish - # TODO: this should behave like #outer_html - assert_equal inner_body, document.match("body").to_html.squish - assert_equal inner_body, document.match("#body").to_html.squish + assert_equal outer_body, document.match("body").to_html.squish + assert_equal outer_body, document.match("#body").to_html.squish end test "should extract whole HTML document" do @@ -124,8 +122,7 @@ class StimulusReflex::HTML::DocumentTest < ActiveSupport::TestCase assert_equal inner_body, document.match("body").inner_html.squish assert_equal inner_body, document.match("#body").inner_html.squish - # TODO: change this to outer_body - assert_equal inner_body, document.match("body").to_html.squish - assert_equal inner_body, document.match("#body").to_html.squish + assert_equal outer_body, document.match("body").to_html.squish + assert_equal outer_body, document.match("#body").to_html.squish end end diff --git a/test/html/fragment_test.rb b/test/html/fragment_test.rb index a007c97b..f6275654 100644 --- a/test/html/fragment_test.rb +++ b/test/html/fragment_test.rb @@ -38,8 +38,8 @@ class StimulusReflex::HTML::FragmentTest < ActiveSupport::TestCase refute fragment.match("body").present? assert_nil fragment.match("body").to_html - assert_equal outer_title, fragment.match("#container").to_html.squish - assert_equal inner_title, fragment.match("#title").to_html.squish + assert_equal raw_html.squish, fragment.match("#container").to_html.squish + assert_equal outer_title, fragment.match("#title").to_html.squish end test "should extract body of a fragment" do @@ -59,8 +59,7 @@ class StimulusReflex::HTML::FragmentTest < ActiveSupport::TestCase assert_equal outer_body, fragment.inner_html.squish assert_equal outer_body, fragment.outer_html.squish - # TODO: change to outer_body - assert_equal inner_body, fragment.match("body").to_html.squish + assert_equal outer_body, fragment.match("body").to_html.squish assert_equal outer_body, fragment.match("body").outer_html.squish assert_equal inner_body, fragment.match("body").inner_html.squish @@ -107,12 +106,10 @@ class StimulusReflex::HTML::FragmentTest < ActiveSupport::TestCase assert_equal outer_body, fragment.outer_html.squish assert_equal outer_body, fragment.inner_html.squish + assert_equal outer_body, fragment.match("body").to_html.squish assert_equal outer_body, fragment.match("body").outer_html.squish assert_equal inner_body, fragment.match("body").inner_html.squish - # TODO: change this to outer_html - assert_equal inner_body, fragment.match("body").to_html.squish - refute fragment.match("#body").present? assert_nil fragment.match("#body").to_html end From cacb89865311fbec0018544265a9962ecbf020d6 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Tue, 23 Aug 2022 02:33:02 +0200 Subject: [PATCH 5/9] Consolidate `Part`, `Document` and `Fragment` into just `HTML::Document` --- lib/stimulus_reflex.rb | 2 - .../broadcasters/page_broadcaster.rb | 8 +- .../broadcasters/selector_broadcaster.rb | 6 +- lib/stimulus_reflex/html/document.rb | 51 +++++++- lib/stimulus_reflex/html/fragment.rb | 11 -- lib/stimulus_reflex/html/part.rb | 58 --------- test/html/document_test.rb | 110 ++++++++++++----- test/html/fragment_test.rb | 116 ------------------ 8 files changed, 131 insertions(+), 231 deletions(-) delete mode 100644 lib/stimulus_reflex/html/fragment.rb delete mode 100644 lib/stimulus_reflex/html/part.rb delete mode 100644 test/html/fragment_test.rb diff --git a/lib/stimulus_reflex.rb b/lib/stimulus_reflex.rb index 0fbd3d47..0c5e65a3 100644 --- a/lib/stimulus_reflex.rb +++ b/lib/stimulus_reflex.rb @@ -13,8 +13,6 @@ require "stimulus_reflex/concern_enhancer" require "stimulus_reflex/configuration" require "stimulus_reflex/callbacks" -require "stimulus_reflex/html/part" -require "stimulus_reflex/html/fragment" require "stimulus_reflex/html/document" require "stimulus_reflex/request_parameters" require "stimulus_reflex/reflex" diff --git a/lib/stimulus_reflex/broadcasters/page_broadcaster.rb b/lib/stimulus_reflex/broadcasters/page_broadcaster.rb index bc9b51b6..b674caf1 100644 --- a/lib/stimulus_reflex/broadcasters/page_broadcaster.rb +++ b/lib/stimulus_reflex/broadcasters/page_broadcaster.rb @@ -4,14 +4,14 @@ module StimulusReflex class PageBroadcaster < Broadcaster def broadcast(selectors, data) reflex.controller.process reflex.params[:action] - fragment = StimulusReflex::HTML::Document.new(reflex.controller.response.body) + document = StimulusReflex::HTML::Document.new(reflex.controller.response.body) - return if fragment.empty? + return if document.empty? - selectors = selectors.select { |s| fragment.match(s).present? } + selectors = selectors.select { |s| document.match(s).present? } selectors.each do |selector| operations << [selector, StimulusReflex.config.morph_operation] - html = fragment.match(selector).inner_html + html = document.match(selector).inner_html cable_ready.send StimulusReflex.config.morph_operation, { selector: selector, html: html, diff --git a/lib/stimulus_reflex/broadcasters/selector_broadcaster.rb b/lib/stimulus_reflex/broadcasters/selector_broadcaster.rb index bcef0089..e209b53c 100644 --- a/lib/stimulus_reflex/broadcasters/selector_broadcaster.rb +++ b/lib/stimulus_reflex/broadcasters/selector_broadcaster.rb @@ -7,8 +7,8 @@ def broadcast(_, data = {}) selectors, html = morph updates = create_update_collection(selectors, html) updates.each do |update| - fragment = StimulusReflex::HTML::Fragment.new(update.html) - match = fragment.match(update.selector) + document = StimulusReflex::HTML::Document.new(update.html) + match = document.match(update.selector) if match.present? operations << [update.selector, StimulusReflex.config.morph_operation] cable_ready.send StimulusReflex.config.morph_operation, { @@ -23,7 +23,7 @@ def broadcast(_, data = {}) operations << [update.selector, StimulusReflex.config.replace_operation] cable_ready.send StimulusReflex.config.replace_operation, { selector: update.selector, - html: fragment.inner_html, + html: update.html.to_s, payload: payload, stimulus_reflex: data.merge(morph: to_sym) } diff --git a/lib/stimulus_reflex/html/document.rb b/lib/stimulus_reflex/html/document.rb index d8a8bec7..e0cc6bde 100644 --- a/lib/stimulus_reflex/html/document.rb +++ b/lib/stimulus_reflex/html/document.rb @@ -1,8 +1,53 @@ +# frozen_string_literal: true + module StimulusReflex module HTML - class Document < Part - def nokogiri_parser(html) - Nokogiri::HTML5::Document.parse(html) + class Document + DEFAULT_HTML_WITHOUT_FORMAT = Nokogiri::XML::Node::SaveOptions::DEFAULT_HTML & ~Nokogiri::XML::Node::SaveOptions::FORMAT + + delegate :element, to: :@document + + def to_html + @document.root.to_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) + end + + def outer_html + @document.root.to_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) + end + + def inner_html + @document.root.inner_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) + end + + def initialize(html) + @document = Nokogiri::HTML5.parse(html.to_s) + @matches = { + "body" => Match.new(@document.at_css("body")) + } + end + + def empty? + @document.content.empty? + end + + def match(selector) + @matches[selector] ||= Match.new(@document.at_css(selector)) + end + + Match = Struct.new(:element) do + delegate :present?, to: :element + + def outer_html + element&.to_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) + end + + def to_html + element&.to_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) + end + + def inner_html + element&.inner_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) + end end end end diff --git a/lib/stimulus_reflex/html/fragment.rb b/lib/stimulus_reflex/html/fragment.rb deleted file mode 100644 index 607e0132..00000000 --- a/lib/stimulus_reflex/html/fragment.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -module StimulusReflex - module HTML - class Fragment < Part - def nokogiri_parser(html) - Nokogiri::HTML.fragment(html) - end - end - end -end diff --git a/lib/stimulus_reflex/html/part.rb b/lib/stimulus_reflex/html/part.rb deleted file mode 100644 index a31855bc..00000000 --- a/lib/stimulus_reflex/html/part.rb +++ /dev/null @@ -1,58 +0,0 @@ -# frozen_string_literal: true - -module StimulusReflex - module HTML - class Part - DEFAULT_HTML_WITHOUT_FORMAT = Nokogiri::XML::Node::SaveOptions::DEFAULT_HTML & ~Nokogiri::XML::Node::SaveOptions::FORMAT - - delegate :element, to: :@fragment - - def to_html - @fragment.to_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) - end - - def outer_html - @fragment.to_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) - end - - def inner_html - @fragment.inner_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) - end - - def nokogiri_parser(html) - raise "not implemented" - end - - def initialize(html) - @fragment = nokogiri_parser(html.to_s) - @matches = { - "body" => Match.new(@fragment.at_css("body")) - } - end - - def empty? - @fragment.content.empty? - end - - def match(selector) - @matches[selector] ||= Match.new(@fragment.at_css(selector)) - end - - Match = Struct.new(:element) do - delegate :present?, to: :element - - def outer_html - element&.to_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) - end - - def to_html - element&.to_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) - end - - def inner_html - element&.inner_html(save_with: DEFAULT_HTML_WITHOUT_FORMAT) - end - end - end - end -end diff --git a/test/html/document_test.rb b/test/html/document_test.rb index 74a9bad9..47c49bc6 100644 --- a/test/html/document_test.rb +++ b/test/html/document_test.rb @@ -2,13 +2,25 @@ require_relative "../test_helper" -class StimulusReflex::HTML::DocumentTest < ActiveSupport::TestCase +class StimulusReflex::HTML::PartTest < ActiveSupport::TestCase test "should handle nil" do document = StimulusReflex::HTML::Document.new(nil) assert_equal "", document.to_html assert_equal "", document.outer_html - assert_equal "", document.inner_html + assert_equal "", document.inner_html + + assert_equal "", document.match("html").to_html + assert_equal "", document.match("html").outer_html + assert_equal "", document.match("html").inner_html + + assert_equal "", document.match("body").to_html + assert_equal "", document.match("body").outer_html + assert_equal "", document.match("body").inner_html + + assert_nil document.match("#selector").to_html + assert_nil document.match("#selector").outer_html + assert_nil document.match("#selector").inner_html end test "should handle empty string" do @@ -16,10 +28,22 @@ class StimulusReflex::HTML::DocumentTest < ActiveSupport::TestCase assert_equal "", document.to_html assert_equal "", document.outer_html - assert_equal "", document.inner_html + assert_equal "", document.inner_html + + assert_equal "", document.match("html").to_html + assert_equal "", document.match("html").outer_html + assert_equal "", document.match("html").inner_html + + assert_equal "", document.match("body").to_html + assert_equal "", document.match("body").outer_html + assert_equal "", document.match("body").inner_html + + assert_nil document.match("#selector").to_html + assert_nil document.match("#selector").outer_html + assert_nil document.match("#selector").inner_html end - test "should accept fragement and build whole document of HTML" do + test "should extract a document of the HTML" do raw_html = <<-HTML

Home#index

@@ -29,57 +53,65 @@ class StimulusReflex::HTML::DocumentTest < ActiveSupport::TestCase document = StimulusReflex::HTML::Document.new(raw_html) inner_title = "Home#index" - outer_title = "

Home#index

" + outer_title = "

#{inner_title}

" inner_container = outer_title outer_container = "
#{inner_container}
" inner_body = outer_container outer_body = "#{inner_body} " - whole_document = "#{outer_body}" + inner_html = "#{outer_body}" + outer_html = "#{inner_html}" - assert_equal outer_body, document.match("body").to_html.squish - assert_equal outer_container, document.match("#container").to_html.squish - assert_equal outer_title, document.match("#title").to_html.squish + assert_equal outer_html, document.to_html.squish + assert_equal outer_html, document.outer_html.squish + assert_equal inner_html, document.inner_html.squish - assert_equal whole_document, document.to_html.squish - assert_equal whole_document, document.inner_html.squish - assert_equal whole_document, document.outer_html.squish + assert_equal outer_html, document.match("html").to_html.squish + assert_equal outer_html, document.match("html").outer_html.squish + assert_equal inner_html, document.match("html").inner_html.squish - assert_equal inner_body, document.match("body").inner_html.squish + assert_equal outer_body, document.match("body").to_html.squish assert_equal outer_body, document.match("body").outer_html.squish + assert_equal inner_body, document.match("body").inner_html.squish - assert_equal inner_container, document.match("#container").inner_html.squish + assert_equal outer_container, document.match("#container").to_html.squish assert_equal outer_container, document.match("#container").outer_html.squish + assert_equal inner_container, document.match("#container").inner_html.squish + assert_equal outer_title, document.match("#title").to_html.squish assert_equal outer_title, document.match("#title").outer_html.squish assert_equal inner_title, document.match("#title").inner_html.squish end test "should extract body of a document" do - raw_html = <<-HTML + raw_body = <<-HTML

Home#index

Find me in app/views/home/index.html.erb

HTML - document = StimulusReflex::HTML::Document.new(raw_html) + document = StimulusReflex::HTML::Document.new(raw_body) inner_body = "

Home#index

Find me in app/views/home/index.html.erb

" outer_body = " #{inner_body} " - whole_document = "#{outer_body}" + inner_html = "#{outer_body}" + outer_html = "#{inner_html}" - assert_equal whole_document, document.to_html.squish - assert_equal whole_document, document.outer_html.squish - assert_equal whole_document, document.inner_html.squish + assert_equal outer_html, document.to_html.squish + assert_equal outer_html, document.outer_html.squish + assert_equal inner_html, document.inner_html.squish - assert_equal outer_body, document.match("body").outer_html.squish - assert_equal outer_body, document.match("#body").outer_html.squish + assert_equal outer_html, document.match("html").to_html.squish + assert_equal outer_html, document.match("html").outer_html.squish + assert_equal inner_html, document.match("html").inner_html.squish - assert_equal inner_body, document.match("#body").inner_html.squish + assert_equal outer_body, document.match("body").to_html.squish + assert_equal outer_body, document.match("body").outer_html.squish assert_equal inner_body, document.match("body").inner_html.squish - assert_equal outer_body, document.match("body").to_html.squish assert_equal outer_body, document.match("#body").to_html.squish + assert_equal outer_body, document.match("#body").outer_html.squish + assert_equal inner_body, document.match("#body").inner_html.squish end test "should extract whole HTML document" do @@ -106,23 +138,33 @@ class StimulusReflex::HTML::DocumentTest < ActiveSupport::TestCase HTML - inner_body = "

Home#index

Find me in app/views/home/index.html.erb

" + document = StimulusReflex::HTML::Document.new(raw_html) + + inner_p = "Find me in app/views/home/index.html.erb" + outer_p = "

#{inner_p}

" + inner_body = "

Home#index

#{outer_p}" outer_body = " #{inner_body} " - expected_html = " StimulusReflex Test #{outer_body}" + inner_html = " StimulusReflex Test #{outer_body}" + outer_html = "#{inner_html}" - document = StimulusReflex::HTML::Document.new(raw_html) + assert_equal outer_html, document.to_html.squish + assert_equal outer_html, document.outer_html.squish + assert_equal inner_html, document.inner_html.squish - assert_equal expected_html, document.to_html.squish - assert_equal expected_html, document.outer_html.squish - assert_equal expected_html, document.inner_html.squish + assert_equal outer_html, document.match("html").to_html.squish + assert_equal outer_html, document.match("html").outer_html.squish + assert_equal inner_html, document.match("html").inner_html.squish + assert_equal outer_body, document.match("body").to_html.squish assert_equal outer_body, document.match("body").outer_html.squish - assert_equal outer_body, document.match("#body").outer_html.squish - assert_equal inner_body, document.match("body").inner_html.squish - assert_equal inner_body, document.match("#body").inner_html.squish - assert_equal outer_body, document.match("body").to_html.squish assert_equal outer_body, document.match("#body").to_html.squish + assert_equal outer_body, document.match("#body").outer_html.squish + assert_equal inner_body, document.match("#body").inner_html.squish + + assert_equal outer_p, document.match("p").to_html.squish + assert_equal outer_p, document.match("p").outer_html.squish + assert_equal inner_p, document.match("p").inner_html.squish end end diff --git a/test/html/fragment_test.rb b/test/html/fragment_test.rb deleted file mode 100644 index f6275654..00000000 --- a/test/html/fragment_test.rb +++ /dev/null @@ -1,116 +0,0 @@ -# frozen_string_literal: true - -require_relative "../test_helper" - -class StimulusReflex::HTML::FragmentTest < ActiveSupport::TestCase - test "should handle nil" do - fragment = StimulusReflex::HTML::Fragment.new(nil) - - assert_equal "", fragment.to_html - assert_equal "", fragment.outer_html - assert_equal "", fragment.inner_html - end - - test "should handle empty string" do - fragment = StimulusReflex::HTML::Fragment.new("") - - assert_equal "", fragment.to_html - assert_equal "", fragment.outer_html - assert_equal "", fragment.inner_html - end - - test "should extract a fragment of the HTML" do - raw_html = <<-HTML -
-

Home#index

-
- HTML - - fragment = StimulusReflex::HTML::Fragment.new(raw_html) - - inner_title = "Home#index" - outer_title = "

#{inner_title}

" - - assert_equal raw_html.squish, fragment.to_html.squish - assert_equal raw_html.squish, fragment.inner_html.squish - assert_equal raw_html.squish, fragment.outer_html.squish - - refute fragment.match("body").present? - assert_nil fragment.match("body").to_html - - assert_equal raw_html.squish, fragment.match("#container").to_html.squish - assert_equal outer_title, fragment.match("#title").to_html.squish - end - - test "should extract body of a fragment" do - raw_body = <<-HTML - -

Home#index

-

Find me in app/views/home/index.html.erb

- - HTML - - fragment = StimulusReflex::HTML::Fragment.new(raw_body) - - inner_body = "

Home#index

Find me in app/views/home/index.html.erb

" - outer_body = " #{inner_body} " - - assert_equal outer_body, fragment.to_html.squish - assert_equal outer_body, fragment.inner_html.squish - assert_equal outer_body, fragment.outer_html.squish - - assert_equal outer_body, fragment.match("body").to_html.squish - assert_equal outer_body, fragment.match("body").outer_html.squish - assert_equal inner_body, fragment.match("body").inner_html.squish - - # Nokogiri fragment's ignore the body, head and html tag if passed into them. That's why this is expected to be nil - # if the body, head or HTML tags matter you should use `StimulusReflex::HTML::Document` instead - refute fragment.match("#body").present? - assert_nil fragment.match("#body").outer_html - assert_nil fragment.match("#body").to_html - assert_nil fragment.match("#body").inner_html - end - - test "should extract whole HTML document" do - raw_body = <<-BODY - -

Home#index

-

Find me in app/views/home/index.html.erb

- - BODY - - raw_html = <<-HTML - - - - StimulusReflex Test - - - - - - - - #{raw_body} - - HTML - - fragment = StimulusReflex::HTML::Fragment.new(raw_html) - - # Nokogiri fragments strip out the body, head and html tag and just stuff every other tag into the body the as well, even if they don't belong there - # This is super unexpected, but that's the way Nokogiri fragements work - inner_body = "StimulusReflex Test

Home#index

Find me in app/views/home/index.html.erb

" - outer_body = " #{inner_body} " - - assert_equal outer_body, fragment.to_html.squish - assert_equal outer_body, fragment.outer_html.squish - assert_equal outer_body, fragment.inner_html.squish - - assert_equal outer_body, fragment.match("body").to_html.squish - assert_equal outer_body, fragment.match("body").outer_html.squish - assert_equal inner_body, fragment.match("body").inner_html.squish - - refute fragment.match("#body").present? - assert_nil fragment.match("#body").to_html - end -end From 3c2846c39aa4d084fa3f4a9afc8df3ce6e74b5a2 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Tue, 23 Aug 2022 02:35:16 +0200 Subject: [PATCH 6/9] fix test name --- test/html/document_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/html/document_test.rb b/test/html/document_test.rb index 47c49bc6..ffa78e6b 100644 --- a/test/html/document_test.rb +++ b/test/html/document_test.rb @@ -2,7 +2,7 @@ require_relative "../test_helper" -class StimulusReflex::HTML::PartTest < ActiveSupport::TestCase +class StimulusReflex::HTML::DocumentTest < ActiveSupport::TestCase test "should handle nil" do document = StimulusReflex::HTML::Document.new(nil) From faf1f78498fbdc5280d107fea576bef1a9461c65 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Tue, 23 Aug 2022 02:36:28 +0200 Subject: [PATCH 7/9] standardize --- test/fragment_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fragment_test.rb b/test/fragment_test.rb index fd9f95d8..5b7d72b3 100644 --- a/test/fragment_test.rb +++ b/test/fragment_test.rb @@ -12,7 +12,7 @@ class StimulusReflex::FragmentTest < ActiveSupport::TestCase fragment = StimulusReflex::Fragment.new(raw_html) - assert_equal fragment.match("#title").to_html.squish, 'Home#index' + assert_equal fragment.match("#title").to_html.squish, "Home#index" end test "should extract top-level element of fragement" do From aadc8dbf3d765bccf77bfd8e6df8eace916e431f Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Tue, 23 Aug 2022 02:40:16 +0200 Subject: [PATCH 8/9] remove duplicate test file --- test/fragment_test.rb | 74 ------------------------------------------- 1 file changed, 74 deletions(-) delete mode 100644 test/fragment_test.rb diff --git a/test/fragment_test.rb b/test/fragment_test.rb deleted file mode 100644 index 5b7d72b3..00000000 --- a/test/fragment_test.rb +++ /dev/null @@ -1,74 +0,0 @@ -# frozen_string_literal: true - -require_relative "test_helper" - -class StimulusReflex::FragmentTest < ActiveSupport::TestCase - test "should extract part of HTML" do - raw_html = <<-HTML -
-

Home#index

-
- HTML - - fragment = StimulusReflex::Fragment.new(raw_html) - - assert_equal fragment.match("#title").to_html.squish, "Home#index" - end - - test "should extract top-level element of fragement" do - raw_html = <<-HTML -
-

Home#index

-
- HTML - - fragment = StimulusReflex::Fragment.new(raw_html) - - assert_equal fragment.to_html.squish, raw_html.squish - assert_equal fragment.match("#container").to_html.squish, '

Home#index

' - end - - test "should extract body of a fragment" do - raw_html = <<-HTML - -

Home#index

-

Find me in app/views/home/index.html.erb

- - HTML - - fragment = StimulusReflex::Fragment.new(raw_html) - - assert_equal fragment.to_html.squish, raw_html.squish - end - - test "should extract whole HTML document" do - raw_body = <<-BODY - -

Home#index

-

Find me in app/views/home/index.html.erb

- - BODY - - raw_html = <<-HTML - - - - StimulusReflex Test - - - - - - - - #{raw_body} - - HTML - - fragment = StimulusReflex::Fragment.new(raw_html) - - assert_equal fragment.match("body").to_html.squish, raw_body.squish - assert_equal fragment.match("#body").to_html.squish, raw_body.squish - assert_equal fragment.to_html.squish, raw_html.squish - end -end From a0725ceb8382e378d0ad3ec03432ce791995baa0 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Tue, 23 Aug 2022 02:52:27 +0200 Subject: [PATCH 9/9] explictly use `Nokogiri::HTML5::Document` --- lib/stimulus_reflex/html/document.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/stimulus_reflex/html/document.rb b/lib/stimulus_reflex/html/document.rb index e0cc6bde..3c6dce35 100644 --- a/lib/stimulus_reflex/html/document.rb +++ b/lib/stimulus_reflex/html/document.rb @@ -20,7 +20,7 @@ def inner_html end def initialize(html) - @document = Nokogiri::HTML5.parse(html.to_s) + @document = Nokogiri::HTML5::Document.parse(html.to_s) @matches = { "body" => Match.new(@document.at_css("body")) }