Typo in rescue for server rendering#963
Typo in rescue for server rendering#963justin808 merged 4 commits intoshakacode:masterfrom AnatoliiD:patch-1
Conversation
|
@railsme this is GREAT! Can you please add a CHANGELOG.md entry and I'll merge this. Reviewed 1 of 1 files at r1. Comments from Reviewable |
|
done 👌 Comments from Reviewable |
|
Reviewed 1 of 1 files at r2. CHANGELOG.md, line 11 at r2 (raw file):
We can make this description a bit more detailed, along the lines of the rubocop comment broke...and this fixed ... plus, please rebase your change on master I can't merge right now due to a conflict... Comments from Reviewable |
|
Should be better now 😃 |
|
@railsme We're not passing Travis |
|
@justin808 #899 broke the build. Should I try to fix it in my PR or wait for fix from the author? 🤔 |
|
If you can fix it, I’d be thrilled. In this pr is fine. |
|
@justin808, I've found that we are using empty strings as default values for not set config options. |
|
@justin808 |
|
Reviewed 1 of 1 files at r3. Comments from Reviewable |
…2580) ## Summary - Reverts #2561 (squash commit 769d120), which was merged after #2568 already landed on master - #2568 + [shakacode/shakapacker#963](shakacode/shakapacker#963) fix the root cause of #2438 — #2561's YAML-parsing fallback is redundant - CHANGELOG entry updated to reference #2568 only ## What's removed All of #2561's additions on top of #2568: - `extract_precompile_hook_from_yaml` / `extract_precompile_hook_from_shakapacker_config` split - `extract_script_from_command` interpreter-prefix stripping - `extract_hash_for_environment` / `current_shakapacker_environment` helpers - `SHAKAPACKER_CONFIG_PATH` constant, `require "erb"`, `require "yaml"` - `rubocop:disable Metrics/ModuleLength` - `GENERATE_PACKS_PATTERN` expansion for `generate_packs_if_needed` - 72 lines of #2561-specific specs ## What remains (from #2568 + shakapacker#963) - `project_root` helper (Rails.root → BUNDLE_GEMFILE → Dir.pwd fallback) - `resolve_hook_script_path` using `project_root` - `require "pathname"` - 3 project_root specs - Shakapacker `Env#current` guard (separate repo, already merged) ## Why #2568 was designed to supersede #2561 — its PR description says so explicitly. Both were merged, resulting in unnecessary complexity (YAML config re-parsing, ERB evaluation, environment fallback chain) that duplicates what Shakapacker already handles after #963. ## Test plan - `bundle exec rspec react_on_rails/spec/react_on_rails/packer_utils_spec.rb` - `bundle exec rubocop react_on_rails/lib/react_on_rails/packer_utils.rb react_on_rails/spec/react_on_rails/packer_utils_spec.rb` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved bin/dev startup reliability with enhanced hook script path resolution during early startup when Rails.root is unavailable. * Removed YAML configuration dependency, streamlining hook detection. * Added validation checks for hook script existence to prevent runtime errors. * Simplified hook script pattern matching for better clarity. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This change is