feat(client): format one of webpack's problems, wherever it is read - #2438
Conversation
🦋 Changeset detectedLatest commit: 57f595d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
`client-src/overlay.js` was the last of it: an adapter whose substance was
turning webpack's error objects into the shape the shared overlay renders, and
a `send({ type })` state machine over `showProblems`/`clear`. Both belong
elsewhere. The formatting is webpack-dev-middleware's `client/problem` now,
and the state machine was standing in for per-source slots that overlay
already has, so the events map onto two calls.
What stays is this package's identity on top of a shared overlay, as options:
the `webpack-dev-server-client-overlay` element id, so anything querying it is
unaffected; the `webpack-dev-server#overlay` Trusted Types policy name, which
a page's CSP allowlists by name; and the `/webpack-dev-server/open-editor`
route that makes a file reference clickable.
`test/client/ReactErrorBoundary.test.js` goes with it — the heuristic it
covers lives in webpack-dev-middleware, which tests it.
Requires webpack/webpack-dev-middleware#2438.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
The overlay took formatted strings only. The middleware formats its own
payloads on the server, so its client never needed anything else — but a
server that sends webpack's error objects to the browser and formats them
there had to write that formatting itself, which is what webpack-dev-server
carried a client file for.
`showProblems` now takes an error object as it comes, and the formatting is
`webpack-dev-middleware/client/problem`: `problemLocation`, `problemBody`,
`problemLine` and `formatProblem`, the last splitting a problem into the
header and body a console wants.
The server's own `formatErrors` builds the same shape, so a build reads the
same way whether the middleware formatted it or a server sent the object over.
Two things it got wrong fall out of that:
* An error webpack names no module for sent a first line holding a single
space. The overlay reads the first line as the heading, so it drew one
with nothing in it. The message goes alone now.
* A module built by loaders reports its whole request as `moduleName`
(`babel-loader!./app.js`), which reads as noise where the file is what
matters. The file comes first, the request follows it, and `file` is used
when webpack sets one — once, not appended to itself.
A unit test asserts the two formatters agree on every shape rather than
trusting them to.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
1e47daa to
57f595d
Compare
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (10)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. WalkthroughThe change adds browser-side formatters for webpack problems and exposes them through the package and overlay declarations. The overlay now accepts strings or problem objects. Server error headings include available module, loader, file, and location details, and omit an empty heading when those details are absent. Tests cover client formatting, overlay rendering, and server output. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The change adds structured problem formatting while preserving supported server inputs. No merge-blocking issue remains; merge after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change primarily broadens diagnostic input and formatting. Structured problems are converted into strings before display, and no new privilege or boundary bypass was demonstrated. Risk remains low rather than minimal because HTML escaping and failure recovery could not be fully verified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2438 +/- ##
==========================================
+ Coverage 96.15% 96.22% +0.06%
==========================================
Files 19 20 +1
Lines 2161 2200 +39
==========================================
+ Hits 2078 2117 +39
Misses 83 83 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Why
webpack-dev-server has no overlay code left after this, which is the point.
The overlay took formatted strings only. The middleware formats its own
payloads on the server, so its client never needed anything else — but a
server that sends webpack's error objects to the browser and formats them
there had to write that formatting itself. That is what webpack-dev-server's
client-src/overlay.jswas: 177 lines whose substance wasproblemLocation/problemBody/problemLine(webpack's error object → theoverlay's string shape) and
formatProblemfor its console. An adapter isstill a client file.
What
showProblemstakes an error object as it comes:and the formatting is a module of its own, for a console as much as an overlay:
problemLocation,problemBody,problemLineandformatProblem, exportedfrom
./client/problemand re-exported from./client/overlayso a consumerdoing both has one import.
Two things the server got wrong
src/hot.jsbuilds the same shape now, so a build reads the same way whetherthe middleware formatted it or a server sent the object over. A unit test
asserts the two agree across every shape rather than trusting them to.
Falling out of that:
" \nmessage"— a first line holding a single space — and the overlay readsthe first line as the heading. The existing test pinned it
(
expect(formatErrors([{ message: "boom" }])).toEqual([" \nboom"])). It isreachable: a real build's
Module not found: Error: Can't resolve 'x'for anentry comes through as
{ loc: "c", message: … }with nomoduleName. Themessage now goes alone.
its whole request as
moduleName, which reads as noise where the file iswhat matters. The file comes first now, the request after it, and
fileisused when webpack sets one — once, not appended to itself
(
./a.js (./a.js)), which is what both the naive version and my own firstattempt produced.
test/problem.test.jscovers that case because it caughtme.
I checked what webpack 5 actually sets before writing these rules, rather than
assuming: for a parse error, a missing module, and a loader failure,
moduleNameis the bare module (./src/c.js) andfileis unset — so theloader and
filebranches are rules for input this project does not produceitself, and matter for a server that sends its own objects.
Proof it does the job
On a dev-server checkout with this packed and installed,
client-src/overlay.jsandtest/client/ReactErrorBoundary.test.js(the Reacterror-boundary heuristic is tested here already) delete outright, and
client-src/index.jscalls this package directly:with the id, the Trusted Types policy name and the open-in-editor route as
configureOverlayoptions,showProblems/clearOverlayin place of thesend({ type })state machine — the per-source slots are what that machinewas for — and
formatProblemfor the console. dev-server's overlay e2e suite:39 pass. That change goes on webpack/webpack-dev-server#5750.
showProblems("warnings", filtered, source)is also called there without alength check, which #2437 makes safe.
State
e2e 138 pass across 12 suites, unit 6909 across 17 (
test/problem.test.jsis18 of them,
formatErrorsgained 5), lint / both typecheck passes / spelling /the precompiled schema check / the full build clean.
test/logging.test.jshasthe two root-only
chmodfailures it has onmainin this container.🤖 Generated with Claude Code
https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
Generated by Claude Code
Summary by CodeRabbit