Skip to content

Commit fae3158

Browse files
Add configurable loading strategy for generated component packs
- Replace `defer_generated_component_packs` with more flexible `generated_component_packs_loading_strategy` - Add support for `:async`, `:defer`, and `:sync` loading strategies - Validate loading strategy based on Shakapacker version - Update helper to support new loading strategy configuration - Add comprehensive specs for the new configuration option
1 parent aefc545 commit fae3158

File tree

4 files changed

+170
-8
lines changed

4 files changed

+170
-8
lines changed

lib/react_on_rails/configuration.rb

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,13 @@ def self.configuration
4343
i18n_output_format: nil,
4444
components_subdirectory: nil,
4545
make_generated_server_bundle_the_entrypoint: false,
46-
defer_generated_component_packs: false,
4746
# forces the loading of React components
4847
force_load: true,
4948
# Maximum time in milliseconds to wait for client-side component registration after page load.
5049
# If exceeded, an error will be thrown for server-side rendered components not registered on the client.
5150
# Set to 0 to disable the timeout and wait indefinitely for component registration.
52-
component_registry_timeout: DEFAULT_COMPONENT_REGISTRY_TIMEOUT
51+
component_registry_timeout: DEFAULT_COMPONENT_REGISTRY_TIMEOUT,
52+
generated_component_packs_loading_strategy: nil
5353
)
5454
end
5555

@@ -64,7 +64,7 @@ class Configuration
6464
:server_render_method, :random_dom_id, :auto_load_bundle,
6565
:same_bundle_for_client_and_server, :rendering_props_extension,
6666
:make_generated_server_bundle_the_entrypoint,
67-
:defer_generated_component_packs, :force_load, :rsc_bundle_js_file,
67+
:generated_component_packs_loading_strategy, :force_load, :rsc_bundle_js_file,
6868
:react_client_manifest_file, :component_registry_timeout
6969

7070
# rubocop:disable Metrics/AbcSize
@@ -76,7 +76,7 @@ def initialize(node_modules_location: nil, server_bundle_js_file: nil, prerender
7676
skip_display_none: nil, generated_assets_dirs: nil,
7777
generated_assets_dir: nil, webpack_generated_files: nil,
7878
rendering_extension: nil, build_test_command: nil,
79-
build_production_command: nil, defer_generated_component_packs: nil,
79+
build_production_command: nil, generated_component_packs_loading_strategy: nil,
8080
same_bundle_for_client_and_server: nil,
8181
i18n_dir: nil, i18n_yml_dir: nil, i18n_output_format: nil, i18n_yml_safe_load_options: nil,
8282
random_dom_id: nil, server_render_method: nil, rendering_props_extension: nil,
@@ -122,8 +122,8 @@ def initialize(node_modules_location: nil, server_bundle_js_file: nil, prerender
122122
self.components_subdirectory = components_subdirectory
123123
self.auto_load_bundle = auto_load_bundle
124124
self.make_generated_server_bundle_the_entrypoint = make_generated_server_bundle_the_entrypoint
125-
self.defer_generated_component_packs = defer_generated_component_packs
126125
self.force_load = force_load
126+
self.generated_component_packs_loading_strategy = generated_component_packs_loading_strategy
127127
end
128128
# rubocop:enable Metrics/AbcSize
129129

@@ -139,6 +139,7 @@ def setup_config_values
139139
# check_deprecated_settings
140140
adjust_precompile_task
141141
check_component_registry_timeout
142+
validate_generated_component_packs_loading_strategy
142143
end
143144

144145
private
@@ -151,6 +152,32 @@ def check_component_registry_timeout
151152
raise ReactOnRails::Error, "component_registry_timeout must be a positive integer"
152153
end
153154

155+
def validate_generated_component_packs_loading_strategy
156+
if PackerUtils.shakapacker_version_requirement_met?([8, 2, 0])
157+
self.generated_component_packs_loading_strategy ||= :async
158+
elsif generated_component_packs_loading_strategy.nil?
159+
msg = <<~MSG
160+
**WARNING** ReactOnRails: Your current version of #{ReactOnRails::PackerUtils.packer_type.upcase_first} \
161+
does not support async script loading which may cause performance issues. Please upgrade to Shakapacker v8.2.0 \
162+
or above to enable async script loading for better performance.
163+
MSG
164+
Rails.logger.warn(msg)
165+
self.generated_component_packs_loading_strategy = :sync
166+
elsif generated_component_packs_loading_strategy == :async
167+
msg = <<~MSG
168+
**ERROR** ReactOnRails: Your current version of #{ReactOnRails::PackerUtils.packer_type.upcase_first} \
169+
does not support async script loading. Please either:
170+
1. Use :sync or :defer loading strategy instead of :async
171+
2. Upgrade to Shakapacker v8.2.0 or above to enable async script loading
172+
MSG
173+
raise ReactOnRails::Error, msg
174+
end
175+
176+
return if [:async, :defer, :sync].include?(generated_component_packs_loading_strategy)
177+
178+
raise ReactOnRails::Error, "generated_component_packs_loading_strategy must be either :async, :defer, or :sync"
179+
end
180+
154181
def check_autobundling_requirements
155182
raise_missing_components_subdirectory if auto_load_bundle && !components_subdirectory.present?
156183
return unless components_subdirectory.present?

lib/react_on_rails/helper.rb

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -422,8 +422,15 @@ def load_pack_for_generated_component(react_component_name, render_options)
422422
is_component_pack_present = File.exist?(generated_components_pack_path(react_component_name))
423423
raise_missing_autoloaded_bundle(react_component_name) unless is_component_pack_present
424424
end
425-
append_javascript_pack_tag("generated/#{react_component_name}",
426-
defer: ReactOnRails.configuration.defer_generated_component_packs)
425+
426+
options = { defer: ReactOnRails.configuration.generated_component_packs_loading_strategy == :defer }
427+
# Old versions of Shakapacker don't support async script tags.
428+
# ReactOnRails.configure already validates if async loading is supported by the installed Shakapacker version.
429+
# Therefore, we only need to pass the async option if the loading strategy is explicitly set to :async
430+
if ReactOnRails.configuration.generated_component_packs_loading_strategy == :async
431+
options[:async] = true
432+
end
433+
append_javascript_pack_tag("generated/#{react_component_name}", **options)
427434
append_stylesheet_pack_tag("generated/#{react_component_name}")
428435
end
429436

spec/dummy/spec/helpers/react_on_rails_helper_spec.rb

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,45 @@ class PlainReactOnRailsHelper
5454
expect(helper).to have_received(:append_stylesheet_pack_tag).with("generated/component_name")
5555
end
5656

57+
context "when async loading is enabled" do
58+
before do
59+
allow(ReactOnRails.configuration).to receive(:generated_component_packs_loading_strategy).and_return(:async)
60+
end
61+
62+
it "appends the async attribute to the script tag" do
63+
original_append_javascript_pack_tag = helper.method(:append_javascript_pack_tag)
64+
begin
65+
# Temporarily redefine append_javascript_pack_tag to handle the async keyword argument.
66+
# This is needed because older versions of Shakapacker (which may be used during testing) don't support async loading,
67+
# but we still want to test that the async option is passed correctly when enabled.
68+
def helper.append_javascript_pack_tag(name, **options)
69+
original_append_javascript_pack_tag.call(name, **options)
70+
end
71+
allow(helper).to receive(:append_javascript_pack_tag)
72+
allow(helper).to receive(:append_stylesheet_pack_tag)
73+
expect { helper.load_pack_for_generated_component("component_name", render_options) }.not_to raise_error
74+
expect(helper).to have_received(:append_javascript_pack_tag).with("generated/component_name", { defer: false, async: true })
75+
expect(helper).to have_received(:append_stylesheet_pack_tag).with("generated/component_name")
76+
ensure
77+
helper.define_singleton_method(:append_javascript_pack_tag, original_append_javascript_pack_tag)
78+
end
79+
end
80+
end
81+
82+
context "when defer loading is enabled" do
83+
before do
84+
allow(ReactOnRails.configuration).to receive(:generated_component_packs_loading_strategy).and_return(:defer)
85+
end
86+
87+
it "appends the defer attribute to the script tag" do
88+
allow(helper).to receive(:append_javascript_pack_tag)
89+
allow(helper).to receive(:append_stylesheet_pack_tag)
90+
expect { helper.load_pack_for_generated_component("component_name", render_options) }.not_to raise_error
91+
expect(helper).to have_received(:append_javascript_pack_tag).with("generated/component_name", { defer: true })
92+
expect(helper).to have_received(:append_stylesheet_pack_tag).with("generated/component_name")
93+
end
94+
end
95+
5796
it "throws an error in development if generated component isn't found" do
5897
allow(Rails.env).to receive(:development?).and_return(true)
5998
expect { helper.load_pack_for_generated_component("nonexisting_component", render_options) }

spec/react_on_rails/configuration_spec.rb

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ module ReactOnRails
265265
end
266266

267267
expect(ReactOnRails::PackerUtils).to have_received(:using_packer?).thrice
268-
expect(ReactOnRails::PackerUtils).to have_received(:shakapacker_version_requirement_met?)
268+
expect(ReactOnRails::PackerUtils).to have_received(:shakapacker_version_requirement_met?).twice
269269
expect(ReactOnRails::PackerUtils).to have_received(:nested_entries?)
270270
end
271271

@@ -277,6 +277,95 @@ module ReactOnRails
277277

278278
expect(ReactOnRails.configuration.random_dom_id).to be(true)
279279
end
280+
281+
describe ".generated_component_packs_loading_strategy" do
282+
context "when using Shakapacker >= 8.2.0" do
283+
before do
284+
allow(ReactOnRails::PackerUtils).to receive(:shakapacker_version_requirement_met?)
285+
.with([8, 2, 0]).and_return(true)
286+
end
287+
288+
it "defaults to :async" do
289+
ReactOnRails.configure {} # rubocop:disable Lint/EmptyBlock
290+
expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:async)
291+
end
292+
293+
it "accepts :async value" do
294+
expect do
295+
ReactOnRails.configure do |config|
296+
config.generated_component_packs_loading_strategy = :async
297+
end
298+
end.not_to raise_error
299+
expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:async)
300+
end
301+
302+
it "accepts :defer value" do
303+
expect do
304+
ReactOnRails.configure do |config|
305+
config.generated_component_packs_loading_strategy = :defer
306+
end
307+
end.not_to raise_error
308+
expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:defer)
309+
end
310+
311+
it "accepts :sync value" do
312+
expect do
313+
ReactOnRails.configure do |config|
314+
config.generated_component_packs_loading_strategy = :sync
315+
end
316+
end.not_to raise_error
317+
expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:sync)
318+
end
319+
320+
it "raises error for invalid values" do
321+
expect do
322+
ReactOnRails.configure do |config|
323+
config.generated_component_packs_loading_strategy = :invalid
324+
end
325+
end.to raise_error(ReactOnRails::Error, /must be either :async, :defer, or :sync/)
326+
end
327+
end
328+
329+
context "when using Shakapacker < 8.2.0" do
330+
before do
331+
allow(ReactOnRails::PackerUtils).to receive(:shakapacker_version_requirement_met?)
332+
.with([8, 2, 0]).and_return(false)
333+
allow(Rails.logger).to receive(:warn)
334+
end
335+
336+
it "defaults to :sync and logs a warning" do
337+
ReactOnRails.configure {} # rubocop:disable Lint/EmptyBlock
338+
expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:sync)
339+
expect(Rails.logger).to have_received(:warn).with(/does not support async script loading/)
340+
end
341+
342+
it "accepts :defer value" do
343+
expect do
344+
ReactOnRails.configure do |config|
345+
config.generated_component_packs_loading_strategy = :defer
346+
end
347+
end.not_to raise_error
348+
expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:defer)
349+
end
350+
351+
it "accepts :sync value" do
352+
expect do
353+
ReactOnRails.configure do |config|
354+
config.generated_component_packs_loading_strategy = :sync
355+
end
356+
end.not_to raise_error
357+
expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:sync)
358+
end
359+
360+
it "raises error for :async value" do
361+
expect do
362+
ReactOnRails.configure do |config|
363+
config.generated_component_packs_loading_strategy = :async
364+
end
365+
end.to raise_error(ReactOnRails::Error, /does not support async script loading/)
366+
end
367+
end
368+
end
280369
end
281370
end
282371

0 commit comments

Comments
 (0)