Fix generated server config lint cleanup

[ihabadham]

Why

Generated server webpack configs imported merge from Shakapacker without using it, and nearby comments incorrectly said the server config performed a merge. The shared commonWebpackConfig already performs the defensive merge/clone, so keeping the extra import and comments creates lint noise and misleading generated code.

Fixes #4791.

What changed

• Remove the unused merge import from the server webpack config template.
• Remove the stale merge comments from the template and dummy app.
• Regenerate all seven affected golden-output variants.
• Record the generator cleanup in the changelog.

Decision

This removes the unused API rather than adding a redundant merge call. Runtime behavior remains unchanged because commonWebpackConfig continues to merge into a fresh object.

Validation

• REGENERATE_GENERATOR_GOLDEN=1 bundle exec rspec spec/react_on_rails/generators/generator_golden_output_spec.rb — 34 examples, 0 failures
• Verification-mode golden-output spec — 34 examples, 0 failures (run by the independent code-review pass)
• OSS RuboCop — passed
• pnpm run lint — passed
• Prettier check — passed
• Pre-commit and pre-push hooks — passed
• git diff --check — passed
• Automated review against origin/main — no findings
• Fresh comment-publication pass — no changes required

Churn notes

The first normal commit attempt was blocked by an outdated Corepack signature key. Corepack was updated to 0.34.6 and pnpm 10.33.4 was activated; hooks were then rerun normally and passed. No hook was bypassed.

Codex decision log

• Scope: generator template, synchronized generated fixtures, dummy app, and changelog only.
• Behavior: no runtime semantic change.
• Confidence: high; source and all golden outputs are synchronized and the focused generator spec passes.

Summary by CodeRabbit

• Bug Fixes

  • Generated server webpack configurations no longer include an unused import or outdated comments.
  • Server configuration behavior remains unchanged, ensuring existing builds continue to work as expected.

• Documentation

  • Added an unreleased changelog entry documenting the configuration cleanup and its unchanged runtime behavior.

Completed-batch audit

Status: Clean — no outstanding findings or follow-ups. Durable receipt.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3575fded-2f03-40a8-b39a-220340bb3dbf

📥 Commits

Reviewing files that changed from the base of the PR and between 1c29441 and f2f095e.

📒 Files selected for processing (1)

• CHANGELOG.md

🚧 Files skipped from review as they are similar to previous changes (1)

• CHANGELOG.md

---

Walkthrough

The generated server webpack configuration no longer imports the unused merge binding or includes obsolete merge comments. The checked-in dummy output reflects the cleanup, and CHANGELOG.md records the fix.

Changes

Server webpack configuration cleanup

Layer / File(s)	Summary
Remove unused merge artifacts react_on_rails/lib/generators/.../serverWebpackConfig.js.tt, react_on_rails/spec/dummy/config/webpack/serverWebpackConfig.js, CHANGELOG.md	The template removes the unused merge import. The dummy configuration removes obsolete comments. The changelog records the generated-output fix.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

• shakacode/react_on_rails#4790: Added golden-output coverage for generated serverWebpackConfig.js.
• shakacode/react_on_rails#4839: Covers generated Webpack configuration output affected by this change.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the lint cleanup in the generated server configuration, which matches the primary change.
Linked Issues check	✅ Passed	The PR removes the unused import and stale comments, regenerates affected golden outputs, and adds the required changelog entry for #4791.
Out of Scope Changes check	✅ Passed	All reported changes support #4791 through template cleanup, generated-output updates, golden-output regeneration, and changelog documentation.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ihabadham/fix/remove-unused-server-webpack-merge

---

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.}

[claude]

Review

Overview

Small, well-scoped cleanup: removes the unused merge import and a stale comment from the generated serverWebpackConfig.js template, propagates the change to the dummy app and all 7 golden-output fixtures, and adds a changelog entry. Fixes #4791.

Correctness ✅

Verified directly:

• commonWebpackConfig.js.tt (line 89) does merge({}, baseClientWebpackConfig, commonOptions) inside commonWebpackConfig(), so every call already returns a fresh, non-shared object. The removed comment in serverWebpackConfig.js.tt ("We need to use 'merge' because...) was describing behavior that lives in commonWebpackConfig, not in the server config itself — it was stale/misplaced, not describing this file's own logic.
• Grepped the full serverWebpackConfig.js.tt template: no other reference to merge remains, so the import removal is safe and not just incidentally unused in one code path.
• No runtime behavior changes — this is purely dead-import + comment cleanup.

Test coverage ✅

• generator_golden_output_spec.rb exists specifically to catch template/fixture drift (per its header comment, added for Generator: add golden-output test for serverWebpackConfig templates to stop fixture drift #4787), and all 7 variants (rspack_base, rspack_pro, rspack_rsc, webpack_base, webpack_base_shakapacker8, webpack_pro, webpack_rsc) plus the dummy app were regenerated consistently in this diff — nothing was missed.
• PR description reports both golden-output regeneration and independent verification-mode runs passing (34/34).

Style / conventions

• Changelog entry follows the existing #### Fixed format and links the issue correctly, consistent with neighboring entries.
• Diff is minimal and mechanical — no unrelated changes bundled in.

Risk

Very low — generator template + fixtures only, no runtime code paths outside code generation are touched, and behavior is unchanged per the commonWebpackConfig analysis above.

No blocking issues found.

[greptile-apps]

Greptile Summary

The PR removes an unused Shakapacker merge import and obsolete merge comments without changing generated server-config behavior.

• Updates the server webpack configuration template and all seven synchronized golden-output variants.
• Cleans up the dummy application’s stale comments.
• Documents the generator cleanup in the changelog.

Confidence Score: 5/5

The PR appears safe to merge because it removes only an unused binding and misleading comments while keeping generated outputs synchronized.

The removed merge binding has no remaining references, the defensive merge remains in commonWebpackConfig, and no executable server-config behavior changes.

Important Files Changed

Filename	Overview
react_on_rails/lib/generators/react_on_rails/templates/base/base/config/webpack/serverWebpackConfig.js.tt	Removes an unused destructured import and inaccurate comments while preserving all executable configuration logic.
react_on_rails/spec/dummy/config/webpack/serverWebpackConfig.js	Removes only stale comments from the dummy application’s server configuration.
react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base/config/webpack/serverWebpackConfig.js	Synchronizes the representative generated webpack fixture with the updated template.
react_on_rails/spec/react_on_rails/fixtures/generated/rspack_rsc/config/rspack/serverWebpackConfig.js	Synchronizes the representative Rspack/RSC generated fixture without changing executable behavior.
CHANGELOG.md	Accurately records the lint-oriented generated-config cleanup under Unreleased fixes.

_{Reviews (1): Last reviewed commit: "Fix generated server config lint cleanup" | Re-trigger Greptile}

[ihabadham]

+ci-status

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CHANGELOG.md`:
- Around line 29-33: Update the changelog entry for Issue 4791 to include the
repository-required pull-request link and author attribution, matching the
formatting used by neighboring CHANGELOG.md entries. Preserve the existing
description and issue link.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 43a9a216-711e-4e3d-ae14-db0742829a24

📥 Commits

Reviewing files that changed from the base of the PR and between 8d08075 and 1c29441.

⛔ Files ignored due to path filters (7)

• react_on_rails/spec/react_on_rails/fixtures/generated/rspack_base/config/rspack/serverWebpackConfig.js is excluded by !**/generated/**
• react_on_rails/spec/react_on_rails/fixtures/generated/rspack_pro/config/rspack/serverWebpackConfig.js is excluded by !**/generated/**
• react_on_rails/spec/react_on_rails/fixtures/generated/rspack_rsc/config/rspack/serverWebpackConfig.js is excluded by !**/generated/**
• react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base/config/webpack/serverWebpackConfig.js is excluded by !**/generated/**
• react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base_shakapacker8/config/webpack/serverWebpackConfig.js is excluded by !**/generated/**
• react_on_rails/spec/react_on_rails/fixtures/generated/webpack_pro/config/webpack/serverWebpackConfig.js is excluded by !**/generated/**
• react_on_rails/spec/react_on_rails/fixtures/generated/webpack_rsc/config/webpack/serverWebpackConfig.js is excluded by !**/generated/**

📒 Files selected for processing (3)

• CHANGELOG.md
• react_on_rails/lib/generators/react_on_rails/templates/base/base/config/webpack/serverWebpackConfig.js.tt
• react_on_rails/spec/dummy/config/webpack/serverWebpackConfig.js

💤 Files with no reviewable changes (1)

• react_on_rails/spec/dummy/config/webpack/serverWebpackConfig.js

[github-actions]

CI Status

Head SHA: 1c29441bfcf9
Changed files: 10
Docs-only heuristic (matches ci-changes-detector metadata paths): no
ready-for-hosted-ci label: absent
force-full-hosted-ci label: absent
Current hosted-CI waiver: not present for this SHA
Automatic release-target hosted mode: inactive
Observed exact-head coverage: modes[missing=9]; successful=0, pending=0, failed=0, missing=9

Only the required gate is active unless hosted CI is requested.

[ihabadham]

+ci-run-hosted

[github-actions]

Hosted CI Requested

Triggered 9 workflow(s) for 1c29441bfcf9.
Skipped 0 workflow(s) with equivalent exact-head coverage.
Mode: optimized hosted CI (path-selected by script/ci-changes-detector).
Added ready-for-hosted-ci, so future commits will keep running optimized hosted CI until +ci-stop-hosted is used.

View progress in the Actions tab.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1c29441bfc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[ihabadham]

QA Evidence

• QA lane: qa-4791; branch ihabadham/fix/remove-unused-server-webpack-merge; worktree /home/ihab/work/shakacode/.worktrees/react_on_rails/ihabadham-fix-remove-unused-server-webpack-merge; claim UNKNOWN; heartbeat UNKNOWN because the private coordination backend is archived/read-only.
• Scope checked: PR Fix generated server config lint cleanup #4840's server template, dummy config, all seven golden variants, unchanged common config, changelog, and exact base-to-head scope.
• Tested at: PR Fix generated server config lint cleanup #4840 head f2f095e673b8429c2d2a15504a8be0552993f979, base 8d08075a0f63a44e0eafce8621d1b32dc7bd1.
• Automated checks: Reviewed implementation evidence: authorized golden regeneration spec passed 34 examples/0 failures; RuboCop, ESLint, Prettier, hooks, and diff check passed. Final metadata-only commit was inspected; git diff --check remains clean.
• Manual checks: Confirmed the template and seven fixtures import only config; stale comments are absent from template, dummy, and exactly seven goldens; every golden calls commonWebpackConfig() once; webpack/rspack pairs remain synchronized; commonWebpackConfig.js.tt still performs merge({}, baseClientWebpackConfig, commonOptions); final changelog includes accurate rationale, issue, PR, and author links; no unrelated paths or generated-output mismatch found.
• User-visible UI change: no
• Visual evidence: not applicable: developer-visible generated configuration only; no UI changed.
• Interaction change: no; not applicable because no user interaction behavior changed.
• Interaction evidence: not applicable: no interaction change.
• Visual fix: no; not applicable because this is generated-source lint cleanup.
• Negative control: not applicable: no visual fix.
• Performance evidence: not applicable: the defensive clone remains in the common config and runtime semantics are unchanged.
• Findings: none
• QA required: yes
• QA required rationale: Generated-output changes require an independent source-to-golden lane.
• QA lane status: satisfied
• Release-blocking status: clear
• Process-gap disposition: not applicable; existing golden regeneration coverage already guards synchronization.

[claude]

Review: Fix generated server config lint cleanup

Overview

Removes the unused merge import and a now-inaccurate comment from the generated serverWebpackConfig.js template, and regenerates all 7 golden-output fixtures + the dummy app to match. Straightforward, low-risk cleanup.

Correctness — verified

• commonWebpackConfig() (in commonWebpackConfig.js.tt) already does merge({}, baseClientWebpackConfig, commonOptions), producing a fresh object on every call. The server config only calls commonWebpackConfig() once and then mutates the returned object directly (serverWebpackConfig.entry = ..., .plugins = ..., etc.) — it never calls merge itself. So the removed import was genuinely dead code, and the removed comment ("We need to use 'merge' because...") was stale/inaccurate for this file. No runtime behavior change. ✅
• Confirmed no leftover const { merge, config } = require('shakapacker') in any fixture or dummy app after the change — all affected variants (webpack/rspack × base/pro/rsc/shakapacker8) were regenerated consistently.
• merge is still correctly imported and used in commonWebpackConfig.js.tt itself (line 4, line 89) — this PR doesn't touch that file, appropriately.

Code quality

• Scope is tight: template + synced golden fixtures + dummy app + changelog. No unrelated changes.
• Changelog entry follows existing conventions (Fixed section, links issue + PR + author).

Minor/nit

• The removed comment block also explained why a fresh object matters (mutable global risk) — that rationale still fully applies, it's just now enforced inside commonWebpackConfig.js.tt rather than at the call site. Might be worth double-checking that commonWebpackConfig.js.tt's own comment ("Copy the object using merge b/c the baseClientWebpackConfig and commonOptions are mutable globals") is prominent enough for future maintainers, since that's now the sole place explaining the mutable-global hazard. Not blocking — just flagging since the explanatory value moved rather than disappeared.

Test coverage

• Golden-output spec (generator_golden_output_spec.rb) covers all 7 regenerated variants per the PR description; this is the right mechanism for verifying generator template + fixture consistency.

Security / performance

• No implications — pure generator-template cleanup, no user-facing runtime code path affected.

Overall: safe, well-scoped, and the core "runtime unchanged" claim checks out against the actual commonWebpackConfig.js.tt implementation. LGTM.

[ihabadham]

Review feedback addressed

Reviewed the complete current review wave through head f2f095e673b8429c2d2a15504a8be0552993f979.

• Handled: CodeRabbit changelog attribution request — verified against neighboring entries, fixed in f2f095e673b8429c2d2a15504a8be0552993f979, replied, and resolved: Fix generated server config lint cleanup #4840 (comment)
• Handled: Codex P2 duplicate changelog attribution request — fixed at the current head, replied, and resolved: Fix generated server config lint cleanup #4840 (comment)
• Declined/no action: Claude's non-blocking suggestion to double-check preservation of the mutable-global rationale. The rationale is already present beside the actual defensive merge in commonWebpackConfig.js.tt; duplicating it at the server call site would recreate the stale/misplaced explanation this PR removes.
• Current-head review: claude-review passed with LGTM; CodeRabbit approved the current head.
• Unresolved review threads: none.

[ihabadham]

+ci-status

[github-actions]

CI Status

Head SHA: f2f095e673b8
Changed files: 10
Docs-only heuristic (matches ci-changes-detector metadata paths): no
ready-for-hosted-ci label: present
force-full-hosted-ci label: absent
Current hosted-CI waiver: not present for this SHA
Automatic release-target hosted mode: inactive
Observed exact-head coverage: modes[missing=9]; successful=0, pending=0, failed=0, missing=9

Optimized hosted CI is enabled for this PR.

[ihabadham]

Completed-batch audit: replay evidence follows.