From 1f3e9217930776cd088535a8b0747497adc5cbd9 Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Mon, 13 Jan 2025 16:31:55 +0200 Subject: [PATCH] pass rsc path to RSC Client Root and move the config to RORP --- lib/react_on_rails/configuration.rb | 11 ++--- lib/react_on_rails/packs_generator.rb | 40 ++++++++++------ .../ruby_embedded_java_script.rb | 4 ++ .../webpack_assets_status_checker.rb | 4 +- node_package/src/ComponentRegistry.ts | 11 ++++- node_package/src/RSCClientRoot.ts | 14 ++++-- node_package/src/ReactOnRails.ts | 5 +- node_package/src/types/index.ts | 6 ++- spec/dummy/spec/packs_generator_spec.rb | 46 +++++++++++++++---- 9 files changed, 101 insertions(+), 40 deletions(-) diff --git a/lib/react_on_rails/configuration.rb b/lib/react_on_rails/configuration.rb index 1a14cfb8f..8dbdc723a 100644 --- a/lib/react_on_rails/configuration.rb +++ b/lib/react_on_rails/configuration.rb @@ -9,7 +9,6 @@ def self.configure end DEFAULT_GENERATED_ASSETS_DIR = File.join(%w[public webpack], Rails.env).freeze - DEFAULT_RSC_RENDERING_URL = "rsc/".freeze def self.configuration @configuration ||= Configuration.new( @@ -43,9 +42,7 @@ def self.configuration make_generated_server_bundle_the_entrypoint: false, defer_generated_component_packs: true, # forces the loading of React components - force_load: false, - auto_load_server_components: true, - rsc_rendering_url: DEFAULT_RSC_RENDERING_URL + force_load: false ) end @@ -60,7 +57,7 @@ class Configuration :same_bundle_for_client_and_server, :rendering_props_extension, :make_generated_server_bundle_the_entrypoint, :defer_generated_component_packs, :rsc_bundle_js_file, - :force_load, :auto_load_server_components, :rsc_rendering_url + :force_load # rubocop:disable Metrics/AbcSize def initialize(node_modules_location: nil, server_bundle_js_file: nil, prerender: nil, @@ -76,7 +73,7 @@ def initialize(node_modules_location: nil, server_bundle_js_file: nil, prerender i18n_dir: nil, i18n_yml_dir: nil, i18n_output_format: nil, random_dom_id: nil, server_render_method: nil, rendering_props_extension: nil, components_subdirectory: nil, auto_load_bundle: nil, force_load: nil, - rsc_bundle_js_file: nil, auto_load_server_components: nil, rsc_rendering_url: nil) + rsc_bundle_js_file: nil) self.node_modules_location = node_modules_location.present? ? node_modules_location : Rails.root self.generated_assets_dirs = generated_assets_dirs self.generated_assets_dir = generated_assets_dir @@ -116,8 +113,6 @@ def initialize(node_modules_location: nil, server_bundle_js_file: nil, prerender self.make_generated_server_bundle_the_entrypoint = make_generated_server_bundle_the_entrypoint self.defer_generated_component_packs = defer_generated_component_packs self.force_load = force_load - self.auto_load_server_components = auto_load_server_components - self.rsc_rendering_url = rsc_rendering_url end # rubocop:enable Metrics/AbcSize diff --git a/lib/react_on_rails/packs_generator.rb b/lib/react_on_rails/packs_generator.rb index 7faccb830..cc4c92f57 100644 --- a/lib/react_on_rails/packs_generator.rb +++ b/lib/react_on_rails/packs_generator.rb @@ -46,30 +46,30 @@ def create_pack(file_path) def first_js_statement_in_code(content) return "" if content.nil? || content.empty? - + start_index = 0 content_length = content.length - + while start_index < content_length # Skip whitespace - while start_index < content_length && content[start_index].match?(/\s/) - start_index += 1 - end - + start_index += 1 while start_index < content_length && content[start_index].match?(/\s/) + break if start_index >= content_length - + current_chars = content[start_index, 2] - + case current_chars - when '//' + when "//" # Single-line comment newline_index = content.index("\n", start_index) return "" if newline_index.nil? + start_index = newline_index + 1 - when '/*' + when "/*" # Multi-line comment - comment_end = content.index('*/', start_index) + comment_end = content.index("*/", start_index) return "" if comment_end.nil? + start_index = comment_end + 2 else # Found actual content @@ -89,9 +89,21 @@ def is_client_entrypoint?(file_path) def pack_file_contents(file_path) registered_component_name = component_name(file_path) - register_as_server_component = ReactOnRails.configuration.auto_load_server_components && !is_client_entrypoint?(file_path) - import_statement = register_as_server_component ? "" : "import #{registered_component_name} from '#{relative_component_path_from_generated_pack(file_path)}';" - register_call = register_as_server_component ? "registerServerComponent(\"#{registered_component_name}\")" : "register({#{registered_component_name}})"; + load_server_components = ReactOnRails::Utils.react_on_rails_pro? && ReactOnRailsPro.configuration.enable_rsc_support + + if load_server_components && !is_client_entrypoint?(file_path) + import_statement = "" + rsc_rendering_url_path = ReactOnRailsPro.configuration.rsc_rendering_url_path + register_call = <<~REGISTER_CALL.strip + registerServerComponent({ + rscRenderingUrlPath: "#{rsc_rendering_url_path}", + }, "#{registered_component_name}") + REGISTER_CALL + else + relative_component_path = relative_component_path_from_generated_pack(file_path) + import_statement = "import #{registered_component_name} from '#{relative_component_path}';" + register_call = "register({#{registered_component_name}})" + end <<~FILE_CONTENT import ReactOnRails from 'react-on-rails'; diff --git a/lib/react_on_rails/server_rendering_pool/ruby_embedded_java_script.rb b/lib/react_on_rails/server_rendering_pool/ruby_embedded_java_script.rb index 99e03a980..ed130cd89 100644 --- a/lib/react_on_rails/server_rendering_pool/ruby_embedded_java_script.rb +++ b/lib/react_on_rails/server_rendering_pool/ruby_embedded_java_script.rb @@ -19,6 +19,10 @@ def reset_pool def reset_pool_if_server_bundle_was_modified return unless ReactOnRails.configuration.development_mode + # RSC (React Server Components) bundle changes are not monitored here since: + # 1. RSC is only supported in the Pro version of React on Rails + # 2. This RubyEmbeddedJavaScript pool is used exclusively in the non-Pro version + # 3. This pool uses ExecJS for JavaScript evaluation which does not support RSC if ReactOnRails::Utils.server_bundle_path_is_http? return if @server_bundle_url == ReactOnRails::Utils.server_bundle_js_file_path diff --git a/lib/react_on_rails/test_helper/webpack_assets_status_checker.rb b/lib/react_on_rails/test_helper/webpack_assets_status_checker.rb index e951df322..f3014af8d 100644 --- a/lib/react_on_rails/test_helper/webpack_assets_status_checker.rb +++ b/lib/react_on_rails/test_helper/webpack_assets_status_checker.rb @@ -50,8 +50,8 @@ def stale_generated_files(files) def all_compiled_assets @all_compiled_assets ||= begin webpack_generated_files = @webpack_generated_files.map do |bundle_name| - if bundle_name == ReactOnRails.configuration.server_bundle_js_file - ReactOnRails::Utils.server_bundle_js_file_path + if bundle_name == ReactOnRails.configuration.react_client_manifest_file + ReactOnRails::Utils.react_client_manifest_file_path else ReactOnRails::Utils.bundle_js_file_path(bundle_name) end diff --git a/node_package/src/ComponentRegistry.ts b/node_package/src/ComponentRegistry.ts index 50624cc4c..8a9c8948b 100644 --- a/node_package/src/ComponentRegistry.ts +++ b/node_package/src/ComponentRegistry.ts @@ -4,6 +4,7 @@ import { type ReactComponentOrRenderFunction, type RenderFunction, type ItemRegistrationCallback, + type RegisterServerComponentOptions, } from './types'; import isRenderFunction from './isRenderFunction'; import CallbackRegistry from './CallbackRegistry'; @@ -49,12 +50,18 @@ export default { }); }, - registerServerComponent(...componentNames: string[]): void { + registerServerComponent(options: RegisterServerComponentOptions, ...componentNames: string[]): void { // eslint-disable-next-line global-require, @typescript-eslint/no-var-requires const RSCClientRoot = (require('./RSCClientRoot') as typeof import('./RSCClientRoot')).default; const componentsWrappedInRSCClientRoot = componentNames.reduce( - (acc, name) => ({ ...acc, [name]: () => React.createElement(RSCClientRoot, { componentName: name }) }), + (acc, name) => ({ + ...acc, + [name]: () => React.createElement(RSCClientRoot, { + componentName: name, + rscRenderingUrlPath: options.rscRenderingUrlPath + }) + }), {} ); this.register(componentsWrappedInRSCClientRoot); diff --git a/node_package/src/RSCClientRoot.ts b/node_package/src/RSCClientRoot.ts index bdf11d77d..81b3f6a01 100644 --- a/node_package/src/RSCClientRoot.ts +++ b/node_package/src/RSCClientRoot.ts @@ -9,13 +9,21 @@ const { use } = React; const renderCache: Record> = {}; -const fetchRSC = ({ componentName }: { componentName: string }) => { +export type RSCClientRootProps = { + componentName: string; + rscRenderingUrlPath: string; +} + +const fetchRSC = ({ componentName, rscRenderingUrlPath }: RSCClientRootProps) => { if (!renderCache[componentName]) { - renderCache[componentName] = RSDWClient.createFromFetch(fetch(`/rsc/${componentName}`)) as Promise; + renderCache[componentName] = RSDWClient.createFromFetch(fetch(`${rscRenderingUrlPath}/${componentName}`)) as Promise; } return renderCache[componentName]; } -const RSCClientRoot = ({ componentName }: { componentName: string }) => use(fetchRSC({ componentName })); +const RSCClientRoot = ({ + componentName, + rscRenderingUrlPath, +}: RSCClientRootProps) => use(fetchRSC({ componentName, rscRenderingUrlPath })); export default RSCClientRoot; diff --git a/node_package/src/ReactOnRails.ts b/node_package/src/ReactOnRails.ts index d02f80da8..64bdfbe90 100644 --- a/node_package/src/ReactOnRails.ts +++ b/node_package/src/ReactOnRails.ts @@ -21,6 +21,7 @@ import type { AuthenticityHeaders, Store, StoreGenerator, + RegisterServerComponentOptions, } from './types'; import reactHydrateOrRender from './reactHydrateOrRender'; @@ -62,8 +63,8 @@ ctx.ReactOnRails = { * When it's rendered, a call will be made to the server to render it. * @param componentNames */ - registerServerComponent(...componentNames: string[]): void { - ComponentRegistry.registerServerComponent(...componentNames); + registerServerComponent(options: RegisterServerComponentOptions, ...componentNames: string[]): void { + ComponentRegistry.registerServerComponent(options, ...componentNames); }, registerStore(stores: { [id: string]: StoreGenerator }): void { diff --git a/node_package/src/types/index.ts b/node_package/src/types/index.ts index beb8b17f5..77645d7bb 100644 --- a/node_package/src/types/index.ts +++ b/node_package/src/types/index.ts @@ -107,6 +107,10 @@ export interface RegisteredComponent { isRenderer: boolean; } +export interface RegisterServerComponentOptions { + rscRenderingUrlPath: string; +} + export type ItemRegistrationCallback = (component: T) => void; interface Params { @@ -156,7 +160,7 @@ export type RenderReturnType = void | Element | Component | Root; export interface ReactOnRails { register(components: { [id: string]: ReactComponentOrRenderFunction }): void; - registerServerComponent(...componentNames: string[]): void; + registerServerComponent(options: RegisterServerComponentOptions, ...componentNames: string[]): void; /** @deprecated Use registerStoreGenerators instead */ registerStore(stores: { [id: string]: StoreGenerator }): void; registerStoreGenerators(storeGenerators: { [id: string]: StoreGenerator }): void; diff --git a/spec/dummy/spec/packs_generator_spec.rb b/spec/dummy/spec/packs_generator_spec.rb index dff16efd1..bc048e3b8 100644 --- a/spec/dummy/spec/packs_generator_spec.rb +++ b/spec/dummy/spec/packs_generator_spec.rb @@ -82,7 +82,7 @@ module ReactOnRails it "generated pack for ComponentWithCommonOnly uses common file for pack" do pack_content = File.read(component_pack) - expect(pack_content).to include("#{component_name}.jsx") + expect(pack_content).to include("#{component_namspec / dummy / spec / packs_generator_spec.rbe}.jsx") expect(pack_content).not_to include("#{component_name}.client.jsx") expect(pack_content).not_to include("#{component_name}.server.jsx") end @@ -297,6 +297,7 @@ def stub_packer_source_path(packer_source_path:, component_name:) context "with simple content" do let(:content) { "const x = 1;" } + it { is_expected.to eq "const x = 1;" } end @@ -309,6 +310,7 @@ def stub_packer_source_path(packer_source_path:, component_name:) const y = 2; JS end + it { is_expected.to eq "const x = 1;" } end @@ -320,6 +322,7 @@ def stub_packer_source_path(packer_source_path:, component_name:) const x = 1; JS end + it { is_expected.to eq "const x = 1;" } end @@ -333,6 +336,7 @@ def stub_packer_source_path(packer_source_path:, component_name:) const x = 1; JS end + it { is_expected.to eq "const x = 1;" } end @@ -341,7 +345,7 @@ def stub_packer_source_path(packer_source_path:, component_name:) <<~JS // First comment - + #{' '} /* multiline comment */ @@ -354,6 +358,7 @@ def stub_packer_source_path(packer_source_path:, component_name:) const x = 1; JS end + it { is_expected.to eq "const x = 1;" } end @@ -364,26 +369,31 @@ def stub_packer_source_path(packer_source_path:, component_name:) /* Another comment */ JS end + it { is_expected.to eq "" } end context "with comment at end of file" do let(:content) { "const x = 1;\n// Final comment" } + it { is_expected.to eq "const x = 1;" } end context "with empty content" do let(:content) { "" } + it { is_expected.to eq "" } end context "with only whitespace" do let(:content) { " \n \t " } + it { is_expected.to eq "" } end context "with statement containing comment-like strings" do let(:content) { 'const url = "http://example.com"; // Real comment' } + # it returns the statement starting from non-space character until the next line even if it contains a comment it { is_expected.to eq 'const url = "http://example.com"; // Real comment' } end @@ -396,6 +406,7 @@ def stub_packer_source_path(packer_source_path:, component_name:) const x = 1; JS end + it { is_expected.to eq "" } end @@ -406,16 +417,19 @@ def stub_packer_source_path(packer_source_path:, component_name:) const x = 1; JS end + it { is_expected.to eq "const x = 1;" } end context "with one line comment with no space after //" do let(:content) { "//const x = 1;" } + it { is_expected.to eq "" } end context "with one line comment with no new line after it" do let(:content) { "// const x = 1" } + it { is_expected.to eq "" } end @@ -428,14 +442,16 @@ def stub_packer_source_path(packer_source_path:, component_name:) const b = 2; JS end + it { is_expected.to eq '"use client";' } end - + context "on top of the file and one line comment" do let(:content) { '"use client"; // const x = 1' } + it { is_expected.to eq '"use client"; // const x = 1' } end - + context "after some one-line comments" do let(:content) do <<~JS @@ -444,6 +460,7 @@ def stub_packer_source_path(packer_source_path:, component_name:) "use client"; JS end + it { is_expected.to eq '"use client";' } end @@ -457,6 +474,7 @@ def stub_packer_source_path(packer_source_path:, component_name:) "use client"; JS end + it { is_expected.to eq '"use client";' } end @@ -470,6 +488,7 @@ def stub_packer_source_path(packer_source_path:, component_name:) "use client"; JS end + it { is_expected.to eq '"use client";' } end @@ -481,14 +500,15 @@ def stub_packer_source_path(packer_source_path:, component_name:) "use client"; JS end - it { is_expected.to eq 'const x = 1;' } + + it { is_expected.to eq "const x = 1;" } end end end - describe "#is_client_entrypoint?", :focus do + describe "#is_client_entrypoint?" do subject { described_class.instance.send(:is_client_entrypoint?, "dummy_path.js") } - + before do allow(File).to receive(:read).with("dummy_path.js").and_return(content) end @@ -496,21 +516,25 @@ def stub_packer_source_path(packer_source_path:, component_name:) context "when file has 'use client' directive" do context "with double quotes" do let(:content) { '"use client";' } + it { is_expected.to be true } end context "with single quotes" do let(:content) { "'use client';" } + it { is_expected.to be true } end context "without semicolon" do let(:content) { '"use client"' } + it { is_expected.to be true } end context "with trailing whitespace" do let(:content) { '"use client" ' } + it { is_expected.to be true } end @@ -523,6 +547,7 @@ def stub_packer_source_path(packer_source_path:, component_name:) "use client"; JS end + it { is_expected.to be true } end end @@ -530,16 +555,19 @@ def stub_packer_source_path(packer_source_path:, component_name:) context "when file does not have 'use client' directive" do context "with empty file" do let(:content) { "" } + it { is_expected.to be false } end context "with regular JS code" do let(:content) { "const x = 1;" } + it { is_expected.to be false } end context "with 'use client' in a comment" do let(:content) { "// 'use client'" } + it { is_expected.to be false } end @@ -550,11 +578,13 @@ def stub_packer_source_path(packer_source_path:, component_name:) "use client"; JS end + it { is_expected.to be false } end context "with similar but incorrect directive" do - let(:content) { 'use client;' } # without quotes + let(:content) { "use client;" } # without quotes + it { is_expected.to be false } end end