docs: remove deprecated devServer.inline references from HMR guide#2479
docs: remove deprecated devServer.inline references from HMR guide#2479
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRemoved all references to the deprecated Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR removes deprecated Changes made:
Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 6bfdfa3 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/guides/hmr-and-hot-reloading-with-the-webpack-dev-server.md (1)
45-54: Minor phrasing nit on line 45.After dropping
and \inline`, the instruction reads: *"sethmrdev_server` property to true"* — two backtick-wrapped identifiers appear back-to-back without connecting prose, which is slightly confusing.✏️ Suggested rewording
-1. In `config/shakapacker.yml` set `hmr` `dev_server` property to true. +1. In `config/shakapacker.yml`, set the `hmr` property under `dev_server` to `true`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/guides/hmr-and-hot-reloading-with-the-webpack-dev-server.md` around lines 45 - 54, The phrase "set `hmr` `dev_server` property to true" is confusing because two backticked identifiers are adjacent; update the wording to a clear form such as "set the `hmr` property in `dev_server` to true" (or "set the `dev_server` `hmr` property to true") so it unambiguously references the `hmr` property within `dev_server`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/guides/hmr-and-hot-reloading-with-the-webpack-dev-server.md`:
- Around line 45-54: The phrase "set `hmr` `dev_server` property to true" is
confusing because two backticked identifiers are adjacent; update the wording to
a clear form such as "set the `hmr` property in `dev_server` to true" (or "set
the `dev_server` `hmr` property to true") so it unambiguously references the
`hmr` property within `dev_server`.
6bfdfa3 to
72055fa
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/building-features/hmr-and-hot-reloading-with-the-webpack-dev-server.md`:
- Line 99: Replace the inconsistent devServer `hmr` phrasing with the YAML key
`dev_server.hmr` so the docs correctly instruct readers what to set in their
config; update the sentence that currently mentions devServer `hmr` to refer to
`dev_server.hmr` (and keep the note that this only affects CSS files) so the key
name matches the rest of the documentation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 20d13cce-62a1-45e9-8282-0cd357f9bec4
📒 Files selected for processing (1)
docs/building-features/hmr-and-hot-reloading-with-the-webpack-dev-server.md
ReviewThe removal of deprecated One concern: the cleanup is incomplete. Two other documentation files still reference
These should be cleaned up in this PR or a follow-up to avoid contradicting the changes being made here. Inline comments below point to each location. Minor: After removing the |
| If you don't configure these two to false, you'll see errors like: | ||
| If you don't configure HMR to false, you'll see errors like: | ||
|
|
||
| - `ReferenceError: window is not defined` (if `hmr` is true) |
There was a problem hiding this comment.
With only one error case remaining, the bullet list feels unnecessary. Consider flattening to prose:
| - `ReferenceError: window is not defined` (if `hmr` is true) | |
| If `hmr` is `true` in this configuration, you will see: `ReferenceError: window is not defined` |
- Remove all references to devServer.inline which no longer exists in webpack 5 - Update configuration examples to only show hmr setting - Simplify error messages section - Fix phrasing for clarity (review feedback) Fixes #1682 Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Use dev_server.hmr notation for consistency with shakapacker.yml - Flatten single-item bullet list to inline prose Co-Authored-By: Claude Opus 4.6 <[email protected]>
72055fa to
7526a27
Compare
|
The second commit addressing the phrasing feedback is good — the HMR guide file is now clean. Still outstanding from my previous review: the
New finding: All four are straightforward one-liner removals/edits. Happy to approve once those are addressed. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/building-features/hmr-and-hot-reloading-with-the-webpack-dev-server.md (1)
32-97:⚠️ Potential issue | 🟠 MajorUpdate configuration documentation to address
dev_server.inlineinconsistency.The HMR guide removes all references to
dev_server.inlineas deprecated, butdocs/configuration/README.md(line 186) still instructs users to set bothdev_server.hmranddev_server.inlineto false when usingsame_bundle_for_client_and_server.If
inlineis deprecated or no longer supported, the configuration documentation must be updated to remove this outdated guidance. Otherwise, users following both guides will receive conflicting instructions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/building-features/hmr-and-hot-reloading-with-the-webpack-dev-server.md` around lines 32 - 97, The docs are inconsistent: the HMR guide removes dev_server.inline but the configuration README still tells users to set dev_server.inline to false; update the configuration README text to remove or mark dev_server.inline as deprecated so it no longer instructs users to toggle dev_server.inline when using same_bundle_for_client_and_server, and instead only reference dev_server.hmr (or explicitly note that dev_server.inline is deprecated/unsupported) so both guides consistently refer to dev_server.hmr and the deprecation state of dev_server.inline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/building-features/hmr-and-hot-reloading-with-the-webpack-dev-server.md`:
- Around line 32-97: The docs are inconsistent: the HMR guide removes
dev_server.inline but the configuration README still tells users to set
dev_server.inline to false; update the configuration README text to remove or
mark dev_server.inline as deprecated so it no longer instructs users to toggle
dev_server.inline when using same_bundle_for_client_and_server, and instead only
reference dev_server.hmr (or explicitly note that dev_server.inline is
deprecated/unsupported) so both guides consistently refer to dev_server.hmr and
the deprecation state of dev_server.inline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1193c48d-5bd5-4598-ab93-9834bab730d1
📒 Files selected for processing (1)
docs/building-features/hmr-and-hot-reloading-with-the-webpack-dev-server.md
|
Addressed the Changes in commit
This aligns |
Review: docs: remove deprecated devServer.inline referencesThe changes here are correct — Two missed locations with stale
These two files should be updated as part of this cleanup to keep the documentation consistent. Minor wording nit (not a blocker): In the HMR doc at the section heading "If you use the same Webpack setup for your server and client bundles", the updated prose ("If |
|
|
||
| - `ReferenceError: window is not defined` (if `hmr` is true) | ||
| - `TypeError: Cannot read property 'prototype' of undefined` (if `inline` is true) | ||
| If `hmr` is `true` in this configuration, you will see: `ReferenceError: window is not defined` |
There was a problem hiding this comment.
The prose here is a bit terse without its prior context. The reader may not realize this constraint only applies to the same-bundle-for-client-and-server scenario described above. Consider:
| If `hmr` is `true` in this configuration, you will see: `ReferenceError: window is not defined` | |
| If `hmr` is `true` when using the same bundle for client and server, you will see: `ReferenceError: window is not defined` |
Add context about "same bundle for client and server" to the HMR error message so readers understand when the ReferenceError applies. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Review: docs: remove deprecated devServer.inline references from HMR guideGood cleanup overall — removing the stale One missed reference: Minor note: The updated error message on line 35 of the HMR guide now only mentions the Otherwise the changes are clean, the yaml snippets and prose are consistent, and the overall direction is correct. |
… doc Aligns with the HMR guide cleanup by removing the last remaining inline: true reference in the docs tree. Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
REVIEW: docs: remove deprecated devServer.inline references from HMR guide Summary: Documentation-only cleanup removing the deprecated Accuracy The Changes look correct
Minor observation In Verdict LGTM. Accurate, minimal, and directly addresses the stale guidance from issue #1682. No runtime behavior is affected. |
## Summary - Add changelog entries for 6 user-visible PRs merged since v16.4.0.rc.6 that were missing from `[Unreleased]` - Update existing #2561 entry to include #2568 contributor credit ### New entries added | Section | PR | Description | |---|---|---| | Added | #2539 | Environment-variable-driven ports in Procfile templates | | Fixed | #2417 | Rspack generator config path fix | | Fixed | #2419 | Precompile hook load-based execution fix | | Fixed | #2577 | `create-react-on-rails-app` validation improvements | | Pro Fixed | #2416 | StreamResponse status fallback fix | | Pro Fixed | #2566 | Empty-string license plan mismatch fix | ### Skipped PRs (not user-visible) Docs (#2406, #2414, #2479, #2494, #2518, #2537, #2544), CI/internal (#2533, #2547, #2555, #2557, #2558, #2564), dependabot (#2387, #2541), dev dependencies (#2559, #2569, #2573). ## Test plan - [ ] Verify changelog formatting matches existing entries - [ ] Verify all user-visible PRs since v16.4.0.rc.6 are covered 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only changelog updates with no runtime or build behavior changes. > > **Overview** > Updates `CHANGELOG.md`’s **[Unreleased]** section to include previously missing user-facing entries: Procfile templates now support env-driven ports, several generator/`bin/dev` precompile-hook and rspack-path fixes are documented, and `create-react-on-rails-app` validation improvements are noted. > > Also adds two Pro fix entries (StreamResponse status fallback and license plan empty-string validation) and updates the existing `bin/dev` precompile-hook entry to credit an additional PR/contributor. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit e75d2b5. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 4.6 <[email protected]>
Summary
Closes #1682
Note
Low Risk
Documentation-only change that updates configuration guidance; no runtime behavior or APIs are modified.
Overview
Updates the HMR guide to remove deprecated
devServer.inline/dev_server.inlineguidance and references, and rewrites the related instructions/error examples to talk only aboutdev_server.hmr.The config snippets and fallback advice are adjusted accordingly (set only
hmr: truefor client-side HMR; ensurehmris false when usingwebpack-dev-serverfor both client/server bundles).Written by Cursor Bugbot for commit 6bfdfa3. Configure here.
Summary by CodeRabbit