Reviewed 2 of 2 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved

Left comment

Reviewed 5 of 5 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Judahmeek)

lib/react_on_rails/server_rendering_pool/ruby_embedded_java_script.rb, line 104 at r2 (raw file):

        def read_bundle_js_code
          server_js_file_path = ReactOnRails::Utils.server_bundle_js_file_path
          server_js_uri = URI.parse(server_js_file_path)

We have another method to see if the server_js_file_path contains a leading http.

Why not check first and branch off that logic?

Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Judahmeek)

lib/react_on_rails/server_rendering_pool/ruby_embedded_java_script.rb, line 107 at r3 (raw file):

          server_js_file = ReactOnRails::Utils.server_bundle_js_file_path
          if ReactOnRails::Utils.server_bundle_path_is_http?
            Net::HTTP.get(URI.parse(server_js_file)).force_encoding("UTF-8")

this is a good change

However, can we add a small comment on the force_encoding?

Maybe break the line into 2 so each intermediate value has a descriptive variable name?

raw_string_value_of_bundle = Net::HTTP.get(URI.parse(server_js_file))

// This is necessary because blah blah
raw_string_value_of_bundle.force_encoding("UTF-8")

Left a comment.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Judahmeek)

Reviewed 5 of 5 files at r4.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Judahmeek)

lib/react_on_rails/utils.rb, line 99 at r4 (raw file):

    def self.bundle_js_file_path(bundle_name)
      if ReactOnRails::WebpackerUtils.using_webpacker? && bundle_name != "manifest.json"
        ReactOnRails::WebpackerUtils.bundle_js_uri_from_webpacker(bundle_name)

webpacker returns either a file path or a URL so renamed to uri to reflect that