Walkthrough

Adds TypeScript (".ts") / ESM awareness for Shakapacker configs: detection and template selection now recognize webpack and rspack TypeScript/ESM patterns, prefer .ts when present, and propagate the detected bundler config path/name through the generator and system checker.

Changes

Sequence Diagram(s)

sequenceDiagram
  participant Generator as InstallGenerator
  participant Checker as SystemChecker
  participant FS as FileSystem
  participant User as Project

  rect rgba(200,230,255,0.5)
  Generator->>Checker: request bundler_main_config_path / standard check
  end

  Checker->>FS: detect_bundler_config_path (check rspack.ts -> rspack.js -> webpack.ts -> webpack.js)
  FS-->>Checker: path or nil
  alt path found
    Checker->>FS: read config content
    FS-->>Checker: file contents
    Checker->>Generator: return detected path + bundler name + stock-pattern match result
  else no path
    Checker-->>Generator: nil (no bundler config)
  end

  Generator->>User: proceed (treat as standard vs customized) based on Checker result

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I hopped through configs, .js and .ts,
Searched for imports, exports, rspack and webpack glee.
Found stock patterns both CJS and ESM,
Chose .ts when present, no replace mayhem.
The rabbit nods: smoother installs for thee!

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.

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

Overall this is a clean, well-motivated fix with good test coverage. A couple of issues to address before merging.

Bug: Wrong PR number in CHANGELOG

The CHANGELOG entry links to PR 2558 but this PR is 2567. See inline comment.

Design: detect_bundler_config_path fixed ordering in system_checker.rb

The method always prefers webpack over rspack (webpack.js -> webpack.ts -> rspack.js -> rspack.ts). In the edge case where a project migrated from webpack to rspack and still has a stale config/webpack/webpack.config.js, the system checker would silently validate the wrong config file.

base_generator.rb's bundler_main_config_path correctly gates on using_rspack? first. The system checker does not have access to that flag, but could check rspack paths before webpack paths (rspack configs are newer and more likely intentional when co-present), or at least document the ordering choice.

Approach inconsistency between the two affected files

base_generator.rb standard_shakapacker_config? uses exact string matching (after normalization), while system_checker.rb standard_shakapacker_config? uses regex matching. This is intentional and correct, but a brief comment in each method explaining the rationale would help future maintainers.

Tests

The new test suite is thorough. Particularly good: the before block in system_checker_spec.rb that defaults all 4 config paths to false, ensuring each test is isolated.

Greptile Summary

This PR extends the install generator and system checker to recognise TypeScript (.ts) Shakapacker config variants introduced in Shakapacker 9.4+. The fix adds ESM regex patterns to standard_shakapacker_config?, updates bundler_main_config_path to prefer .ts files when they exist, and broadens shakapacker_configured? and detect_bundler_config_path to cover all four config variants (webpack.config.{js,ts} and rspack.config.{js,ts}).

Key remaining issues:

• Template mismatch: All three template() call-sites in copy_webpack_main_config still use the JavaScript template unconditionally. When a TypeScript config is detected and replaced, the generated file at the .ts path will contain CommonJS JavaScript content rather than idiomatic TypeScript, silently downgrading TypeScript-first projects.
• Ordering inconsistency: detect_bundler_config_path (system checker) prefers .js over .ts, while bundler_main_config_path (generator) prefers .ts over .js. In the unlikely edge case where both coexist, they would operate on different files.
• Stale debug hint: suggest_webpack_inspection hardcodes config/webpack/webpack.config.js and module.exports syntax, which is misleading for TypeScript or rspack users seeking debugging guidance.
• CHANGELOG typo: The PR link references Pin npm shakapacker version to match gem in example generation #2558 instead of the current PR Recognize TypeScript (.ts) Shakapacker config variants #2567.

The core goal — stopping false "customised config" prompts for TypeScript Shakapacker users — is achieved and well-tested. Tests are comprehensive for the new patterns and cover path detection for .ts variants. However, the logic gap where the generator replaces a .ts config with CommonJS JavaScript content should be addressed before merging.

Confidence Score: 3/5

• Safe to merge for detection improvements, but the JS-template-to-TS-path issue should be addressed to avoid silently downgrading TypeScript projects.
• The PR successfully achieves its core goal — stopping false "customised config" prompts for TypeScript Shakapacker users. Detection logic is sound, pattern matching is comprehensive, and tests are thorough. However, there is a logic gap where the generator replaces a detected .ts config file using the CommonJS JavaScript template, which downgrades TypeScript-first setups. This is a real regression that should be fixed before merging. Additionally, there are three minor but worth-noting issues: ordering inconsistency between checker and generator, stale hardcoded debugging instructions, and a CHANGELOG PR link typo.
• react_on_rails/lib/generators/react_on_rails/base_generator.rb — the template selection logic needs to be dynamic (choose between .js.tt and .ts.tt based on the detected config path).

_{Last reviewed commit: 39b0163}

All three review comments addressed in 08c0895:

1. CHANGELOG PR number: Fixed link from PR 2558 → PR 2567
2. detect_bundler_config_path ordering: Reordered to check rspack paths first (rspack is newer, more likely the active bundler when both co-exist). Added explanatory comment.
3. Regex vs exact matching comment: Added clarifying comment to system_checker.rb's standard_shakapacker_config? explaining the intentional difference from base_generator.rb's exact-string approach.

Code Review

Overall this is a clean, well-tested addition. The core logic for detecting .ts configs and recognizing ESM syntax is correct. A few issues worth addressing:

1. Stale hardcoded webpack.config.js paths in suggest_webpack_inspection

suggest_webpack_inspection (system_checker.rb ~L327, L338) still references config/webpack/webpack.config.js literally in its user-facing hint strings. Rspack users who trigger this code path will see webpack-specific debug instructions that don't apply to their setup. This method should either receive the detected config_path as an argument and interpolate it, or update the references to be more generic.

2. check_webpack_config_content messages say "Webpack config" for all bundlers

Lines ~366 and ~370 in system_checker.rb emit messages like "✅ Webpack config includes React on Rails environment configuration" and "⚠️ Standard Shakapacker webpack config detected." These are shown even when the detected file is an rspack config. The method is now bundler-agnostic but its output strings are not.

3. detect_bundler_config_path priority ordering may surprise users

The comment justifies checking rspack paths first with "rspack is more likely to be the active bundler (it's the newer default)" — but the vast majority of existing installs use webpack. The consequence is: if someone has both a leftover rspack config and an active webpack config, the checker silently audits the wrong file.

base_generator.rb resolves this correctly by using using_rspack? (option-driven) rather than file-system heuristics. Consider passing a bundler hint to detect_bundler_config_path, or at least reversing the priority so webpack-first matches align with what most users have.

4. Missing comment-banner variants for ESM configs in shakapacker_esm_default_configs

The CJS config list (shakapacker_cjs_default_configs) includes both the comment-banner version (with the // See the shakacode/shakapacker README... header) and the plain version for each template. The new ESM configs only include the plain versions. If Shakapacker 9.4+ generates .ts configs with that comment header, standard_shakapacker_config? in base_generator.rb will return false and incorrectly treat the file as customized. Worth either checking what Shakapacker 9.4 actually generates or adding comment-banner variants defensively.

5. No test coverage for detect_bundler_config_path

The new detect_bundler_config_path method has no dedicated unit test. Given that its priority ordering is a deliberate design choice (rspack before webpack), a test verifying "when both rspack and webpack configs exist, rspack path is returned" would prevent accidental future regression — and also force a conversation about whether that priority is intentional.

Also addressed CodeRabbit's outside-diff suggestion in 6d3cac2: check_webpack_config_content now derives bundler name from the config path, so react_on_rails:doctor shows "rspack" or "webpack" accurately instead of always saying "Webpack".

PR Review: Recognize TypeScript (.ts) Shakapacker config variants

Overall, this is a well-scoped fix for a real pain point: Shakapacker 9.4+ TypeScript configs being incorrectly treated as customized. The approach is sound and the test coverage is solid. A few issues worth addressing:

Bug: suggest_webpack_inspection gives wrong advice for rspack users

check_webpack_configuration now correctly handles rspack configs, but it calls suggest_webpack_inspection unconditionally. That method still has hardcoded webpack-specific advice: it instructs users to add debugger; before module.exports in config/webpack/webpack.config.js (wrong for rspack), and recommends yarn add --dev webpack-bundle-analyzer with edits to config/webpack/webpack.config.js for bundle analysis (webpack-bundle-analyzer is webpack-specific; rspack uses @rspack/plugin-bundle-analyzer). An rspack user running the system checker will see these incorrect instructions.

Fragile: ESM template exact matching does not handle double-quoted imports

shakapacker_esm_default_configs uses single-quoted import paths. normalize_config_content only strips comments and collapses whitespace — it does not normalize quote style. If Shakapacker or a user's Prettier config generates the TypeScript config with double quotes, content_matches_template? will not match and the generator will treat a stock config as customized. TypeScript/ESM projects with Prettier commonly use singleQuote: false.

Minor: Priority assumption in detect_bundler_config_path is weak

The comment says "rspack is more likely to be the active bundler (it's the newer default)" to justify checking rspack paths first. This does not hold for a project mid-migration where both config files coexist with webpack being active. The heuristic is a reasonable default, but the comment overstates its reliability.

Review

Overall: This is a clean, well-scoped fix. The core logic — adding ESM/TS patterns to shakapacker_default_configs and making shakapacker_configured?/check_webpack_configuration bundler-agnostic — is correct and well-tested. Good use of exact-string matching in base_generator.rb vs. looser regex matching in system_checker.rb; the explanatory comment at line 454 calling out the intentional difference is appreciated.

One practical leftover worth addressing:

suggest_webpack_inspection is invoked unconditionally from check_webpack_configuration for all bundler configs, yet it still hardcodes webpack-specific file paths in its tips. An rspack user running rails react_on_rails:doctor will see advice pointing to config/webpack/webpack.config.js — a path that does not exist in their project. This is a pre-existing issue, but the PR is the first to route rspack configs through this method, so now is the right moment to address it (see inline comment).

Minor notes (non-blocking):

• bundler_config_file_exists? and detect_bundler_config_path check paths in opposite order (webpack-first vs rspack-first). Different purposes so it is fine, but a short comment on bundler_config_file_exists? noting that order is insignificant there would prevent future confusion.
• The test "rejects customized TypeScript config with user modifications" in install_generator_spec.rb uses resolve!.extensions! which is not valid TypeScript syntax. The assertion is still correct (exact-match normalisation rejects the content regardless), but using a realistic modification like appending a plugin would make the test intent clearer.

Review

Solid fix for a real user-facing pain point. The structure is clean and the test coverage is thorough. A few items worth addressing:

1. bundle_analyzer_available? false-positive for rspack — the method only checks for webpack-bundle-analyzer. If an rspack project has that package in package.json (e.g., from a webpack migration), suggest_webpack_inspection takes the if bundle_analyzer_available? branch and shows "This opens the configured bundle analyzer in your browser" even though webpack-bundle-analyzer does not support rspack. The bundler_name guard only exists in the elsif branch.

2. YAML parsed with regex in configured_assets_bundler — the regex won't match values arriving via YAML anchors/aliases (<<: *default) or values that only exist inside environment-specific blocks. YAML.safe_load would handle these cases and is equally safe given the existing rescue StandardError guard.

3. Minor template style inconsistency — webpack.config.js.tt and rspack.config.js.tt use an if/else pattern; the two new .ts.tt templates use early-return (no else). Worth aligning for consistency.