Revert: Fix component registration race condition (#1972)

[justin808]

Summary

Reverts commit d1a8a1a which changed the default component loading strategy from :sync to :defer.

This reverts the fix for the component registration race condition that was introduced in #1972.

Changes

• Reverts the defer loading strategy back to the original behavior
• Restores async loading support when available
• Updates tests to reflect the original expectations

---

This change is 

Summary by CodeRabbit

Release Notes

• New Features

  • Intelligent component pack loading now adapts based on your Shakapacker version and Redux configuration.

• Bug Fixes

  • Enhanced validation with clearer warning messages when async loading is unsupported; guides users to upgrade or use alternative strategies.

• Tests

  • Updated test expectations to reflect refined default loading strategies across Shakapacker versions.

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

The PR modifies generated component pack loading strategy handling to support async loading when Shakapacker version permits. The configuration validator now checks PackerUtils support and defaults to async for newer versions, sync for older versions with warnings, while retaining deprecation handling. Templates and tests are updated accordingly.

Changes

Cohort / File(s)	Summary
Configuration logic lib/react_on_rails/configuration.rb	Enhanced validate_generated_component_packs_loading_strategy with multi-branch logic: defaults to :async when PackerUtils.supports_async_loading? is true, :sync with warning when not supported, and raises error if explicitly :async but unsupported. Retains deprecation handling for defer_generated_component_packs.
Dummy app initialization spec/dummy/config/initializers/react_on_rails.rb	Explicitly sets config.generated_component_packs_loading_strategy = :defer and removes comments describing default behavior reliance.
Dummy app template spec/dummy/app/views/layouts/application.html.erb	Replaces static defer-only script tag with conditional logic that uses defer: true when uses_redux_shared_store? is true, otherwise async: true. Adds inline documentation.
Configuration tests spec/react_on_rails/configuration_spec.rb	Updates test expectations: Shakapacker ≥ 8.2.0 defaults to :async, Shakapacker < 8.2.0 defaults to :sync with warning log verification via Rails.logger.

Sequence Diagram(s)

sequenceDiagram
    participant Config as Configuration<br/>(validate method)
    participant Utils as PackerUtils
    participant Log as Rails.logger
    
    Config->>Utils: supports_async_loading?
    alt Async supported (v8.2.0+)
        Utils-->>Config: true
        Note over Config: Default to :async
    else Async not supported (< v8.2.0)
        Utils-->>Config: false
        alt Strategy is nil
            Config->>Log: warn async not supported
            Note over Config: Default to :sync
        else Strategy is explicitly :async
            Config-->>Config: raise error
        end
    end
    
    Note over Config: Validate final strategy is one of<br/>:async, :defer, or :sync

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–25 minutes

• lib/react_on_rails/configuration.rb: Multi-branch conditional logic with PackerUtils integration requires careful validation of all paths (supported/unsupported versions, explicit vs. implicit strategies).
• spec/react_on_rails/configuration_spec.rb: Test expectations reflect version-dependent defaults; verify warning message assertions match implementation.
• Template change (application.html.erb): Conditional rendering logic depends on uses_redux_shared_store?; ensure both branches produce expected output.

Possibly related PRs

• Fix invalid version checker warnings and add pre-release comparison #1742: Modifies PackerUtils version parsing and comparison logic that this PR depends on for PackerUtils.supports_async_loading? checks.
• Add configurable loading strategy for generated component packs #1712: Adds original generated_component_packs_loading_strategy configuration and related load_pack_for_generated_component async/defer handling.
• Fix component registration race condition with defer loading strategy #1972: Modifies the same validation method but with opposing async-first defaults, creating potential merge conflict or behavioral divergence.

Suggested labels

review-needed

Suggested reviewers

• AbanoubGhadban
• Judahmeek

Poem

🐰 Async or defer, the choice is now clear!
When Shakapacker's ready, we load without fear.
Version checks guide us down the right path,
No more warnings or mysterious wrath!
The strategy blooms with each config file's care. 🌱

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch justin808/revert-defer-race-fix

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 93c15ef and 0bc9f0d.

📒 Files selected for processing (4)

• lib/react_on_rails/configuration.rb (2 hunks)
• spec/dummy/app/views/layouts/application.html.erb (1 hunks)
• spec/dummy/config/initializers/react_on_rails.rb (1 hunks)
• spec/react_on_rails/configuration_spec.rb (2 hunks)

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}