feat(client): let progress choose between the badge and a bar - #2426
Conversation
webpack-dev-server's `client.progress` takes `"linear"` and `"circular"` as well as a boolean, and renders a visibly different indicator for each. This package took a boolean only, so those two values would have silently collapsed into one shape — a value that used to mean something quietly meaning nothing is worse than a value that errors. `"circular"` is the badge this package has always shown, and stays what `true` selects, so nothing changes for anyone already using it. `"linear"` renders a thin bar across the top of the viewport, measured by the percentage the progress payloads carry and sweeping when there is no percentage to measure — the same choice the badge makes between its ring and its pulsing dot. Built through the CSSOM and the Web Animations API, with no `<style>` element, so a strict `style-src` still allows it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
🦋 Changeset detectedLatest commit: 2a77370 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 |
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review. WalkthroughThe progress option now accepts boolean values and the Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The linear option selects the top bar, and the inspected build and motion-preference paths reveal no actionable issue blocking merge. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The new option changes how build progress appears without an identified change to privileged operations or security boundaries. A possible listener-cleanup issue during repeated rebuilds presents limited browser-lifecycle risk. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 576f88fd-6078-412c-9191-1fb4aa5a9516
📒 Files selected for processing (4)
.changeset/progress-linear.mdclient-src/index.jsclient-src/indicator.jstest/e2e/indicator.test.js
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review.
Review finding. The bar's sweep ran regardless of `prefers-reduced-motion`, which webpack-dev-server's linear indicator honors in its own stylesheet — so the shape this PR adds for parity was missing the part of it that matters most to the people it matters to. The badge's pulsing dot ignored the preference too, which predates this branch. Both go through one guard now rather than leaving the older one wrong: nothing starts while motion is declined, anything already running stops if the preference changes mid-build, and the listener that watches for it is dropped when the indicator goes away. Without a sweep the bar says a build is running by sitting still at full width, so the state is still visible. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: c7a5b616-5d26-4a57-8414-7152a2c914d8
📒 Files selected for processing (2)
client-src/indicator.jstest/e2e/indicator.test.js
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review.
Two review findings, both in the reduced-motion guard added a commit ago. `matchMedia` returns a new `MediaQueryList` for every call, so removing the listener from a freshly built one called `removeEventListener` and changed nothing: the object that actually held it kept it, one per build, for the life of the page. Verified in the browser the suite runs rather than from the specification alone. The query the listener was registered on is kept and removed from. Cancelling a sweep left `barAnimation` pointing at the cancelled animation and the bar frozen at the 40% a sweep starts from — so it read as progress that had stalled, and every later call returned early on the stale reference and could never put it right. Declining motion now clears the reference and puts the bar at the width it uses when it cannot move. The first test written for the listener passed against the bug, because it counted calls rather than what stayed attached; it tracks listeners per query object now, and fails when the removal is aimed at the wrong one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2426 +/- ##
==========================================
- Coverage 96.30% 95.91% -0.40%
==========================================
Files 17 17
Lines 1894 1957 +63
==========================================
+ Hits 1824 1877 +53
- Misses 70 80 +10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Summary
webpack-dev-server's
client.progresstakes"linear"and"circular"as well as a boolean, and renders a visibly different indicator for each. This package took a boolean only.That matters for the step where dev-server stops shipping its own client: both values would have arrived here and silently collapsed into one shape. A value that used to mean something quietly meaning nothing is worse than a value that errors — nobody gets a warning, the UI just changes.
"circular"is the badge this package has always shown, and stays whattrueselects, so nothing changes for anyone already using it."linear"renders a thin bar across the top of the viewport: measured by the percentage theprogresspayloads carry, and sweeping when there is no percentage to measure — the same choice the badge already makes between its ring and its pulsing dot.Built through the CSSOM and the Web Animations API with no
<style>element, like the rest of the indicator, so a strictstyle-srcstill allows it. Choosing a type while something is on screen removes it and lets the next event rebuild in the new shape, since the two are different elements; which sources are mid-build is kept, so the indicator still hides only once they have all finished.This is independent of #2425 — either can land first.
What kind of change does this PR introduce?
feature
Did you add tests for your changes?
Yes, one end-to-end case:
progress=linearis pinned to the top edge and spans the viewport rather than sitting in the badge's bottom-right corner, and its filled part is driven by the reported percentage. Sampled from inside the page, because a build can finish faster than a round trip from the test.Teeth-checked: forcing the type back to
"circular"fails it.Verified locally: end-to-end 103/103, non-browser 6811 passed, lint and both typechecks clean.
test/logging.test.jsfails 74/74 on a cleanmaintoo, unrelated to this.Does this PR introduce a breaking change?
No.
progress: trueandprogress: falsebehave exactly as before; the two new values are additions.If relevant, what needs to be documented once your changes are merged or what have you already documented?
The
progressoption's accepted values, once the README documents the client options. Not included here.Use of AI
AI-assisted (Claude Code). Used to compare this indicator against webpack-dev-server's, implement the linear variant and its test, and verify it by reverting the type selection and confirming the test fails.
🤖 Generated with Claude Code
https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
Generated by Claude Code
Summary by CodeRabbit
New Features
progress: "linear";trueand"circular"continue to show the badge.Tests