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

Code Review Summary

This PR upgrades webpack-cli from v4 to v6.0.1 to maintain compatibility with webpack-dev-server v5. Overall, the changes are well-executed and focused, addressing the core compatibility issue. Here's my detailed feedback:

✅ Strengths

1. Correct dependency upgrade: The upgrade from webpack-cli v4 to v6.0.1 is the right approach for webpack-dev-server v5 compatibility
2. Proper cleanup: Removing @webpack-cli/serve is correct - it was bundled into webpack-cli starting from v5+
3. Deprecated config removal: Removing https: false from shakapacker.yml is appropriate - this setting became deprecated
4. Consistent changes: All three dummy apps (open-source, pro, and execjs-compatible) were updated consistently
5. Lockfile updates: pnpm-lock.yaml files properly reflect the dependency changes

⚠️ Issues Found

1. CRITICAL: Documentation needs update

The documentation file docs/building-features/hmr-and-hot-reloading-with-the-webpack-dev-server.md still references the deprecated https: false setting:

# Line 53 in the docs - SHOULD BE REMOVED
dev_server:
  https: false  # ❌ This is deprecated
  host: localhost
  port: 3035

Action required: Update the documentation to remove this deprecated config option.

🔍 Testing Recommendations

As per CLAUDE.md guidelines, before merging you should:

1. Test clean installation (MANDATORY)

# In each dummy app:
cd react_on_rails/spec/dummy
rm -rf node_modules
pnpm install --frozen-lockfile

cd ../../react_on_rails_pro/spec/dummy
rm -rf node_modules
pnpm install --frozen-lockfile

cd ../execjs-compatible-dummy
rm -rf node_modules
pnpm install --frozen-lockfile

2. Test webpack-dev-server works

# Verify dev server starts without errors
cd react_on_rails/spec/dummy
bin/shakapacker-dev-server
# Should start without deprecation warnings or errors

3. Test build process

# From root
pnpm run build
pnpm run yalc:publish

4. Run full test suite

# From root
bundle exec rubocop
rake

📋 Before Merging Checklist

• Update documentation to remove https: false example
• Test clean install in all 3 dummy apps
• Verify webpack-dev-server starts without warnings
• Run full test suite locally
• Wait for CI to pass
• Monitor CI after merge (per CLAUDE.md master health monitoring)

🎯 Overall Assessment

Status: Approve with minor documentation fix required

The code changes are solid and correct. The only blocker is updating the documentation to match the new configuration. Once the docs are updated and tests pass, this is ready to merge.

📚 Additional Context

From webpack-cli v5+ release notes:

• @webpack-cli/serve, @webpack-cli/info, and @webpack-cli/configtest were integrated into the core webpack-cli package
• The https: false option in dev-server config is unnecessary (false is the default)
• Minimum Node.js version for webpack-cli v6 is 18.12.0

This upgrade aligns the project with current webpack ecosystem standards.

Greptile Overview

Greptile Summary

This PR upgrades webpack-cli from v4 to v6.0.1 to resolve compatibility issues with webpack-dev-server v5. The changes are applied consistently across three dummy applications used for testing.

Changes Made

1. webpack-cli upgrade (v4 → v6.0.1): Updated in all three dummy apps (react_on_rails, react_on_rails_pro, and execjs-compatible-dummy)
2. @webpack-cli/serve removal: Removed from react_on_rails_pro/spec/dummy/package.json as it's now bundled in webpack-cli v5+
3. Deprecated config removal: Removed https: false from all three shakapacker.yml files (obsolete configuration option)
4. Lock file updates: pnpm-lock.yaml files updated for the two apps that have them (execjs-compatible-dummy doesn't maintain a lock file)

Architecture Context

The upgrade path is straightforward:

• webpack-cli v6 bundles @webpack-cli/serve v3 internally (no longer needs separate installation)
• webpack-dev-server v5 requires webpack-cli v5+ for proper compatibility
• The https: false option in shakapacker.yml is deprecated and defaults to false when omitted
• All changes are isolated to test/dummy applications and don't affect production code

Verification

All changes are consistent across the three dummy applications:

• Version specifiers use ^6.0.1 consistently
• Configuration changes mirror each other
• Lock files reflect proper dependency resolution with webpack-cli v6 and its bundled dependencies

Confidence Score: 5/5

• This PR is safe to merge with minimal risk - it's a straightforward dependency upgrade with proper version compatibility
• The changes are well-contained, consistent across all affected files, and follow best practices. The upgrade resolves a known compatibility issue between webpack-cli v4 and webpack-dev-server v5. All three dummy applications received identical treatment, lock files are properly updated, and deprecated configuration options are correctly removed. No breaking changes identified - webpack-cli v6 maintains backward compatibility for the command-line interface used by Shakapacker's wrapper scripts.
• No files require special attention - all changes are straightforward dependency upgrades

Important Files Changed

File Analysis

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant Rails as Rails App
    participant Shakapacker as Shakapacker
    participant WebpackCLI as webpack-cli v6
    participant WebpackDevServer as webpack-dev-server v5
    
    Note over Dev,WebpackDevServer: Before: webpack-cli v4 + webpack-dev-server v5 (incompatible)
    
    Dev->>Rails: bin/shakapacker-dev-server
    Rails->>Shakapacker: DevServerRunner.run()
    Shakapacker->>WebpackCLI: Execute webpack-cli v4
    WebpackCLI--xWebpackDevServer: ❌ Version mismatch
    
    Note over Dev,WebpackDevServer: After: webpack-cli v6 + webpack-dev-server v5 (compatible)
    
    Dev->>Rails: bin/shakapacker-dev-server
    Rails->>Shakapacker: DevServerRunner.run()
    Note over Shakapacker: Reads shakapacker.yml<br/>(https: false removed)
    Shakapacker->>WebpackCLI: Execute webpack-cli v6
    Note over WebpackCLI: @webpack-cli/serve v3<br/>bundled internally
    WebpackCLI->>WebpackDevServer: ✅ Start dev server
    WebpackDevServer-->>Dev: Hot Module Replacement ready

PR Review: Fix webpack-cli compatibility with webpack-dev-server v5

Summary

This PR upgrades webpack-cli from v4 to v6.0.1 to ensure compatibility with webpack-dev-server v5. The changes include:

• Upgrading webpack-cli to v6.0.1 across all dummy apps
• Removing the deprecated @webpack-cli/serve package (now bundled in webpack-cli v5+)
• Removing deprecated https: false configuration from shakapacker.yml files
• Pinning shakapacker to 9.4.0 in generated examples

Code Quality & Best Practices ✅

Positive aspects:

1. Consistent versioning: webpack-cli is consistently upgraded to ^6.0.1 across all package.json files
2. Proper cleanup: @webpack-cli/serve has been completely removed from dependencies (it's now bundled in webpack-cli v5+)
3. Configuration cleanup: Removed deprecated https: false from all shakapacker.yml configs (3 files)
4. Good inline documentation: The comment in shakapacker_examples.rake clearly explains why shakapacker is pinned to 9.4.0

Potential Issues & Recommendations ⚠️

1. CRITICAL: Missing Testing per CLAUDE.md Requirements

According to .claude/docs/testing-build-scripts.md, when modifying package.json or build configs, you MUST manually test:

# Step 1: Test clean install (MOST CRITICAL)
rm -rf node_modules
pnpm install --frozen-lockfile

# Step 2: Test build scripts
pnpm run build

# Step 3: Test package-specific scripts
pnpm run prepack
pnpm run yalc:publish

The PR checklist shows:

• pnpm install succeeds in all dummy apps
• CI passes ❌

Please also verify:

• pnpm run build tested
• pnpm run yalc:publish tested (critical per CLAUDE.md)
• RuboCop passed (bundle exec rubocop)

2. Version Pinning Strategy Needs Clarification

The PR pins shakapacker to exactly 9.4.0 in generated examples with this comment:

Pin to 9.4.0 to ensure gem and npm package versions match (shakapacker 9.5.0 gem was released but npm package version detection has issues)

Questions:

• Is this a temporary workaround or permanent solution?
• Should this be documented in a changelog or issue tracker?
• Will this cause problems when shakapacker 9.5.x npm package is released?

3. Breaking Change Consideration

The removal of https: false from shakapacker.yml is noted as removing a "deprecated" option. However:

• Is this truly deprecated in webpack-dev-server v5, or just unsupported?
• Users upgrading may need migration guidance
• Consider documenting this in CHANGELOG.md if it's user-facing

Performance & Security ✅

• No performance concerns identified
• No security issues identified
• Upgrading to latest stable versions is good security practice

Test Coverage ⚠️

Missing verification:

1. CI hasn't passed yet
2. Should verify that webpack-dev-server still works correctly with the new webpack-cli

Recommended tests:

• Manual test: Start webpack-dev-server in dummy apps
• Verify HMR (Hot Module Replacement) still works
• Run relevant RSpec integration tests

Minor Suggestions

1. Update CHANGELOG.md: This is a user-facing change (webpack-cli upgrade) and should be documented
2. Document testing: Add to PR description which tests were run locally to verify the changes work
3. Shakapacker pinning: Consider filing an issue to track the shakapacker 9.5.0 compatibility problem

Overall Assessment

Code Quality: Good ✅

• Changes are logical and consistent
• Good cleanup of deprecated dependencies

Testing: Needs Verification ⚠️

• CI must pass before merge
• Build scripts should be tested per CLAUDE.md guidelines

Documentation: Could be improved ℹ️

• Consider adding CHANGELOG entry

Recommendation

This PR is on the right track, but per CLAUDE.md requirements, please:

1. ✅ Ensure all mandatory tests pass (CI, RuboCop, build scripts)
2. ✅ Document testing performed (especially pnpm run yalc:publish)
3. ✅ Verify the shakapacker pinning is intentional and won't cause issues
4. 💭 Consider adding a CHANGELOG entry for the webpack-cli upgrade

Once CI passes and testing is verified, this should be good to merge!