test(react-grab): coverage suite — unit + e2e sweep - #510
Open
aidenybai wants to merge 10 commits into
Open
Conversation
Adds a FreezeHookHarness fixture (useReducer, useTransition, useSyncExternalStore, and a context consumer/provider) plus a freeze-hooks spec asserting each counter holds frozen during prompt-mode freeze and resumes normal updates after unfreeze. This drives the freeze-updates.ts pause/resume paths the useState-only freeze-updates spec never reached, notably context-dependency pause/resume.
…tle waits - Move FreezeHookHarness into its own module (mirrors PerfGrid), keeping App.tsx a composition root and relocating the external-store singleton. - Correct the misleading "bypasses useState" comments (transition/context counters update via useState) and document the synthetic-click quirk. - Add settle waits so the freeze-hold assertion can catch a leaked update and the post-unfreeze baseline is stable; guard readCount against a missing node; narrow the combined test to the simultaneity claim.
Covers the border-width, z-index, and opacity branches (plus spacing edge cases and the unknown-key path) that the edit-panel e2e flow never drove, including the 1px suffix-less border form and the off-step/out-of-range rejections.
Closes several reliable, deterministic coverage gaps that were stuck at low/zero because the e2e suite either inlines or never exercises them: - errors.ts: instantiate all five classes, assert name/message and SelectorTimeoutError.timeoutMs (was 27%, no class instantiated) - clamp-to-range.ts / format-color-label.ts: cover the inlined zeros - safe-decode-uri-component.ts: cover the malformed-escape catch path - parse-package-name.ts: exercise resolvePackageName scoped fallback, CDN-URL parsing, vite chunk rejection, .pnpm skip (lifts func coverage) - parse-activation-key.ts: cover both matcher branches + getModifiers
The e2e suite skips the Next fixture, so these document-driven helpers were structurally untested. Stub `document` in node to cover their branches: - is-next-project-runtime.ts: __NEXT_DATA__ (Pages) and nextjs-portal (App Router) detection, the no-document and no-marker negatives, and the memoize/revalidate cache contract. - get-next-base-path.ts: the configured-basePath prefix, the root /_next/ and missing-script empty cases, and first-call memoization (fresh module per branch since the cache has no reset hook).
Targets branch gaps the e2e suite never feeds the right input types for: - css-property-bounds.ts: every property family incl. the z-index and percent-unit arms, plus the value-scaled size ceiling (both Math.max sides) - parse-any-color.ts: hex expansion, the transparent keyword, and the hand-rolled oklch() converter across all hue units / alpha / percent forms (canvas-free paths; the no-canvas null fall-through is asserted too) - css-baseline-measurement.ts: every isDefaultByHeuristic arm and the isDefaultByBaseline color / layout-dependent / snapshot-compare branches
Covers the two highest-leverage gaps in the keydown handler that no spec reached before: - Keyboard context-menu trigger (tryHandleContextMenuKey): the ContextMenu key and Shift+F10 open the menu on the hovered selection, and both are ignored while inactive. - Window-refocus grace period (didWindowJustRegainFocus): activation keys are suppressed for the grace window after the window regains focus, then work again once it elapses. The modifier is held before the focus event so only the single activation keydown must land inside the window, keeping the assertion robust under parallel runs.
The fiber debug-stack recovery path (enrichServerFrameLocations and its owner-stack extraction) had no unit coverage. Stub bippy's fiber traversal to exercise the no-unresolved/no-server-frame early exits, the name-matched file/line/column merge (first-match-wins, server-URL-only filtering), and the no-match passthrough. Also cover symbolicateServerFrames' <unknown> methodName and null line/column fallbacks for bare server frames.
freeze-updates.ts (the dispatcher-patch freeze/replay path, the report's top fragile-internals gap) had no unit coverage. Mock bippy's fiber/renderer surface and the recoverable-error logger to drive the real freezeUpdates end-to-end: - hook-queue buffering: reads masked to null while frozen, every buffered update replayed in chain order on resume (exercises all four mergePendingChains arms via single/single and multi/multi merges), plus the pre-existing pending chain captured at freeze time. - context-dependency freezing: value masked while frozen, buffered write applied on resume. - dispatcher replay order: store callbacks -> transitions -> state updates. - recovery: a throwing buffered update is swallowed/logged and replay continues; a scheduleUpdate failure in the post-resume microtask flush is swallowed/logged. - lifecycle guards: re-freeze and double-resume are no-ops.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Jun 29, 2026
This was referenced Jun 29, 2026
commit: |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 18 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/react-grab/tests/format-color-label.test.ts">
<violation number="1" location="packages/react-grab/tests/format-color-label.test.ts:12">
P3: `.toUpperCase()` on `"#00000000"` is a no-op (only digits + `#`). The test claims to verify case-insensitive handling, but no letter characters exist to convert. Use a hex string with letters (e.g. `"#ff000000"`) to actually exercise the case-insensitive path.</violation>
</file>
<file name="packages/react-grab/e2e/freeze-hooks.spec.ts">
<violation number="1" location="packages/react-grab/e2e/freeze-hooks.spec.ts:83">
P2: Combined test verifies freeze but never verifies resume — if a multi-hook interaction bug (e.g. context-dependency interfere with useReducer/useTransition replay) prevents clean unfreeze, this test won't catch it.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| await assertFreezeHoldsThenResumes(reactGrab, "context-count", "context-increment"); | ||
| }); | ||
|
|
||
| test("all hook counters stay frozen together during one freeze cycle", async ({ reactGrab }) => { |
Contributor
There was a problem hiding this comment.
P2: Combined test verifies freeze but never verifies resume — if a multi-hook interaction bug (e.g. context-dependency interfere with useReducer/useTransition replay) prevents clean unfreeze, this test won't catch it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/e2e/freeze-hooks.spec.ts, line 83:
<comment>Combined test verifies freeze but never verifies resume — if a multi-hook interaction bug (e.g. context-dependency interfere with useReducer/useTransition replay) prevents clean unfreeze, this test won't catch it.</comment>
<file context>
@@ -0,0 +1,104 @@
+ await assertFreezeHoldsThenResumes(reactGrab, "context-count", "context-increment");
+ });
+
+ test("all hook counters stay frozen together during one freeze cycle", async ({ reactGrab }) => {
+ const reducerBefore = await readCount(reactGrab, "reducer-count");
+ const transitionBefore = await readCount(reactGrab, "transition-count");
</file context>
|
|
||
| it("labels the fully transparent hex as 'transparent' (case-insensitive)", () => { | ||
| expect(formatColorLabel("#00000000")).toBe(EDIT_TRANSPARENT_COLOR_LABEL); | ||
| expect(formatColorLabel("#00000000".toUpperCase())).toBe(EDIT_TRANSPARENT_COLOR_LABEL); |
Contributor
There was a problem hiding this comment.
P3: .toUpperCase() on "#00000000" is a no-op (only digits + #). The test claims to verify case-insensitive handling, but no letter characters exist to convert. Use a hex string with letters (e.g. "#ff000000") to actually exercise the case-insensitive path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/tests/format-color-label.test.ts, line 12:
<comment>`.toUpperCase()` on `"#00000000"` is a no-op (only digits + `#`). The test claims to verify case-insensitive handling, but no letter characters exist to convert. Use a hex string with letters (e.g. `"#ff000000"`) to actually exercise the case-insensitive path.</comment>
<file context>
@@ -0,0 +1,18 @@
+
+ it("labels the fully transparent hex as 'transparent' (case-insensitive)", () => {
+ expect(formatColorLabel("#00000000")).toBe(EDIT_TRANSPARENT_COLOR_LABEL);
+ expect(formatColorLabel("#00000000".toUpperCase())).toBe(EDIT_TRANSPARENT_COLOR_LABEL);
+ });
+
</file context>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ae051c9. Configure here.
| }; | ||
| video.addEventListener("seeked", handleSeeked); | ||
| video.currentTime = time; | ||
| }); |
There was a problem hiding this comment.
Video seek promise may hang
High Severity
seekTo waits only for a seeked event after setting video.currentTime. When the target time equals the current time (common for jank at clip start or repeated sample times), browsers often skip seeked, so the promise never resolves, extractKeyframes hangs, and trace grab stays stuck with isGrabbing true.
Reviewed by Cursor Bugbot for commit ae051c9. Configure here.
| .filter((event) => event.kind === "jank") | ||
| .map((event) => | ||
| Math.max(0, (event.timestamp - (performance.now() - CLIP_DURATION_MS)) / 1000), | ||
| ); |
There was a problem hiding this comment.
Jank keyframes use wrong timeline
Medium Severity
Jank keyframe seek times are computed from a fixed CLIP_DURATION_MS window ending at grab time, but the WebM blob only spans time since recording started (up to that cap). Before the buffer is full, offsets are too large and clamp to the end of the video, so exported keyframes miss the actual jank moments.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ae051c9. Configure here.
Contributor
There was a problem hiding this comment.
12 issues found across 11 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/react-grab/src/trace/jank-detector.ts">
<violation number="1" location="packages/react-grab/src/trace/jank-detector.ts:57">
P2: Use `entry.startTime` for long-animation-frame timestamps. Otherwise exported trace timelines can place long frames after the actual jank, especially for expensive frames.</violation>
<violation number="2" location="packages/react-grab/src/trace/jank-detector.ts:90">
P2: Use `entry.startTime` for longtask timestamps. Current code shifts timeline offsets by the task/callback delay, making exported traces and key timing misleading for long main-thread blocks.</violation>
</file>
<file name="packages/react-grab/src/trace/replay-buffer.ts">
<violation number="1" location="packages/react-grab/src/trace/replay-buffer.ts:57">
P1: Stop the acquired display stream if MediaRecorder setup/start fails. Otherwise a failed Record attempt can leave browser screen sharing running after the UI reports capture failed.</violation>
<violation number="2" location="packages/react-grab/src/trace/replay-buffer.ts:61">
P2: Do not treat the first MediaRecorder Blob as metadata-only. Pinning it into every later clip makes long recordings export stale frames from capture start instead of only the rolling window.</violation>
</file>
<file name="packages/react-grab/src/trace/trace-mode.ts">
<violation number="1" location="packages/react-grab/src/trace/trace-mode.ts:51">
P2: Jank keyframe seek times are computed assuming the video blob spans the full `CLIP_DURATION_MS` window. However, if recording started less than `CLIP_DURATION_MS` ago, the video is shorter and these offsets will exceed the actual video duration. They get clamped to `duration - 0.01` in `extractKeyframes`, causing all keyframes to capture the same final frame instead of the actual jank moments. Consider computing offsets relative to the actual recording start time or the resolved video duration.</violation>
<violation number="2" location="packages/react-grab/src/trace/trace-mode.ts:77">
P2: Do not report “clip copied” after clipboard.writeText fails. Track write success and set a fallback status so users know the transcript is not on the clipboard.</violation>
<violation number="3" location="packages/react-grab/src/trace/trace-mode.ts:78">
P2: Reset isGrabbing in a finally block. A failed clip build currently wedges trace mode until it is restarted.</violation>
<violation number="4" location="packages/react-grab/src/trace/trace-mode.ts:100">
P1: Guard the pending Record start, not just active recording. Double-clicking Record before getDisplayMedia resolves can leak an untracked screen-capture stream/recorder.</violation>
</file>
<file name="packages/react-grab/src/trace/trace-ui.ts">
<violation number="1" location="packages/react-grab/src/trace/trace-ui.ts:42">
P3: Restoring buttons with `all: unset` removes the browser focus indicator without adding a replacement. Add a `:focus-visible` style so keyboard users can see which trace control is active.</violation>
<violation number="2" location="packages/react-grab/src/trace/trace-ui.ts:86">
P3: The shortcut hint only shows the Mac modifier even though the handler also supports Ctrl+Shift+C. Show both modifiers or choose by platform to avoid misleading Windows/Linux users.</violation>
</file>
<file name="packages/react-grab/src/trace/extract-keyframes.ts">
<violation number="1" location="packages/react-grab/src/trace/extract-keyframes.ts:8">
P2: Decode failures leak the created object URL because `video` remains `null` in the caller's `finally`. Revoke the URL from the `error` path in `loadVideo`.</violation>
<violation number="2" location="packages/react-grab/src/trace/extract-keyframes.ts:15">
P1: `seekTo` can hang when asked to seek to the current position, so `extractKeyframes` may never return for a first keyframe at `0s`. Resolve immediately when the target time already matches `video.currentTime`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| initializationChunk = null; | ||
| mediaChunks = []; | ||
|
|
||
| recorder = new MediaRecorder(stream, { mimeType }); |
Contributor
There was a problem hiding this comment.
P1: Stop the acquired display stream if MediaRecorder setup/start fails. Otherwise a failed Record attempt can leave browser screen sharing running after the UI reports capture failed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/trace/replay-buffer.ts, line 57:
<comment>Stop the acquired display stream if MediaRecorder setup/start fails. Otherwise a failed Record attempt can leave browser screen sharing running after the UI reports capture failed.</comment>
<file context>
@@ -0,0 +1,116 @@
+ initializationChunk = null;
+ mediaChunks = [];
+
+ recorder = new MediaRecorder(stream, { mimeType });
+ recorder.ondataavailable = (event) => {
+ if (event.data.size === 0) return;
</file context>
| replayBuffer.stop(); | ||
| return; | ||
| } | ||
| void begin(); |
Contributor
There was a problem hiding this comment.
P1: Guard the pending Record start, not just active recording. Double-clicking Record before getDisplayMedia resolves can leak an untracked screen-capture stream/recorder.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/trace/trace-mode.ts, line 100:
<comment>Guard the pending Record start, not just active recording. Double-clicking Record before getDisplayMedia resolves can leak an untracked screen-capture stream/recorder.</comment>
<file context>
@@ -0,0 +1,111 @@
+ replayBuffer.stop();
+ return;
+ }
+ void begin();
+ });
+ ui.onGrabClip(() => void grabClip());
</file context>
Comment on lines
+15
to
+23
| const seekTo = (video: HTMLVideoElement, time: number): Promise<void> => | ||
| new Promise((resolve) => { | ||
| const handleSeeked = () => { | ||
| video.removeEventListener("seeked", handleSeeked); | ||
| resolve(); | ||
| }; | ||
| video.addEventListener("seeked", handleSeeked); | ||
| video.currentTime = time; | ||
| }); |
Contributor
There was a problem hiding this comment.
P1: seekTo can hang when asked to seek to the current position, so extractKeyframes may never return for a first keyframe at 0s. Resolve immediately when the target time already matches video.currentTime.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/trace/extract-keyframes.ts, line 15:
<comment>`seekTo` can hang when asked to seek to the current position, so `extractKeyframes` may never return for a first keyframe at `0s`. Resolve immediately when the target time already matches `video.currentTime`.</comment>
<file context>
@@ -0,0 +1,72 @@
+ });
+ });
+
+const seekTo = (video: HTMLVideoElement, time: number): Promise<void> =>
+ new Promise((resolve) => {
+ const handleSeeked = () => {
</file context>
Suggested change
| const seekTo = (video: HTMLVideoElement, time: number): Promise<void> => | |
| new Promise((resolve) => { | |
| const handleSeeked = () => { | |
| video.removeEventListener("seeked", handleSeeked); | |
| resolve(); | |
| }; | |
| video.addEventListener("seeked", handleSeeked); | |
| video.currentTime = time; | |
| }); | |
| const seekTo = (video: HTMLVideoElement, time: number): Promise<void> => { | |
| if (Math.abs(video.currentTime - time) < 0.001) return Promise.resolve(); | |
| return new Promise((resolve) => { | |
| const handleSeeked = () => { | |
| video.removeEventListener("seeked", handleSeeked); | |
| resolve(); | |
| }; | |
| video.addEventListener("seeked", handleSeeked); | |
| video.currentTime = time; | |
| }); | |
| }; |
| if (frameDelta > JANK_FRAME_THRESHOLD_MS) { | ||
| recordEvent({ | ||
| kind: "jank", | ||
| timestamp: performance.now(), |
Contributor
There was a problem hiding this comment.
P2: Use entry.startTime for long-animation-frame timestamps. Otherwise exported trace timelines can place long frames after the actual jank, especially for expensive frames.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/trace/jank-detector.ts, line 57:
<comment>Use `entry.startTime` for long-animation-frame timestamps. Otherwise exported trace timelines can place long frames after the actual jank, especially for expensive frames.</comment>
<file context>
@@ -0,0 +1,130 @@
+ if (frameDelta > JANK_FRAME_THRESHOLD_MS) {
+ recordEvent({
+ kind: "jank",
+ timestamp: performance.now(),
+ durationMs: frameDelta,
+ droppedFrames: Math.round(frameDelta / TARGET_FRAME_BUDGET_MS) - 1,
</file context>
|
|
||
| observeEntries("longtask", (entry) => | ||
| entry.duration >= LONG_TASK_THRESHOLD_MS | ||
| ? { kind: "longtask", timestamp: performance.now(), durationMs: entry.duration } |
Contributor
There was a problem hiding this comment.
P2: Use entry.startTime for longtask timestamps. Current code shifts timeline offsets by the task/callback delay, making exported traces and key timing misleading for long main-thread blocks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/trace/jank-detector.ts, line 90:
<comment>Use `entry.startTime` for longtask timestamps. Current code shifts timeline offsets by the task/callback delay, making exported traces and key timing misleading for long main-thread blocks.</comment>
<file context>
@@ -0,0 +1,130 @@
+
+ observeEntries("longtask", (entry) =>
+ entry.duration >= LONG_TASK_THRESHOLD_MS
+ ? { kind: "longtask", timestamp: performance.now(), durationMs: entry.duration }
+ : null,
+ );
</file context>
|
|
||
| ui.flashCopied(); | ||
| ui.setStatus(`clip copied — ${keyframes.length} keyframe(s), ${events.length} event(s)`); | ||
| isGrabbing = false; |
Contributor
There was a problem hiding this comment.
P2: Reset isGrabbing in a finally block. A failed clip build currently wedges trace mode until it is restarted.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/trace/trace-mode.ts, line 78:
<comment>Reset isGrabbing in a finally block. A failed clip build currently wedges trace mode until it is restarted.</comment>
<file context>
@@ -0,0 +1,111 @@
+
+ ui.flashCopied();
+ ui.setStatus(`clip copied — ${keyframes.length} keyframe(s), ${events.length} event(s)`);
+ isGrabbing = false;
+ };
+
</file context>
Comment on lines
+8
to
+12
| video.src = URL.createObjectURL(blob); | ||
| video.addEventListener("loadeddata", () => resolve(video), { once: true }); | ||
| video.addEventListener("error", () => reject(new Error("Clip video failed to decode.")), { | ||
| once: true, | ||
| }); |
Contributor
There was a problem hiding this comment.
P2: Decode failures leak the created object URL because video remains null in the caller's finally. Revoke the URL from the error path in loadVideo.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/trace/extract-keyframes.ts, line 8:
<comment>Decode failures leak the created object URL because `video` remains `null` in the caller's `finally`. Revoke the URL from the `error` path in `loadVideo`.</comment>
<file context>
@@ -0,0 +1,72 @@
+ const video = document.createElement("video");
+ video.muted = true;
+ video.preload = "auto";
+ video.src = URL.createObjectURL(blob);
+ video.addEventListener("loadeddata", () => resolve(video), { once: true });
+ video.addEventListener("error", () => reject(new Error("Clip video failed to decode.")), {
</file context>
Suggested change
| video.src = URL.createObjectURL(blob); | |
| video.addEventListener("loadeddata", () => resolve(video), { once: true }); | |
| video.addEventListener("error", () => reject(new Error("Clip video failed to decode.")), { | |
| once: true, | |
| }); | |
| const objectUrl = URL.createObjectURL(blob); | |
| video.src = objectUrl; | |
| video.addEventListener("loadeddata", () => resolve(video), { once: true }); | |
| video.addEventListener( | |
| "error", | |
| () => { | |
| URL.revokeObjectURL(objectUrl); | |
| reject(new Error("Clip video failed to decode.")); | |
| }, | |
| { once: true }, | |
| ); |
| const jankOffsets = events | ||
| .filter((event) => event.kind === "jank") | ||
| .map((event) => | ||
| Math.max(0, (event.timestamp - (performance.now() - CLIP_DURATION_MS)) / 1000), |
Contributor
There was a problem hiding this comment.
P2: Jank keyframe seek times are computed assuming the video blob spans the full CLIP_DURATION_MS window. However, if recording started less than CLIP_DURATION_MS ago, the video is shorter and these offsets will exceed the actual video duration. They get clamped to duration - 0.01 in extractKeyframes, causing all keyframes to capture the same final frame instead of the actual jank moments. Consider computing offsets relative to the actual recording start time or the resolved video duration.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/trace/trace-mode.ts, line 51:
<comment>Jank keyframe seek times are computed assuming the video blob spans the full `CLIP_DURATION_MS` window. However, if recording started less than `CLIP_DURATION_MS` ago, the video is shorter and these offsets will exceed the actual video duration. They get clamped to `duration - 0.01` in `extractKeyframes`, causing all keyframes to capture the same final frame instead of the actual jank moments. Consider computing offsets relative to the actual recording start time or the resolved video duration.</comment>
<file context>
@@ -0,0 +1,111 @@
+ const jankOffsets = events
+ .filter((event) => event.kind === "jank")
+ .map((event) =>
+ Math.max(0, (event.timestamp - (performance.now() - CLIP_DURATION_MS)) / 1000),
+ );
+ const keyframes = videoBlob ? await extractKeyframes(videoBlob, jankOffsets) : [];
</file context>
|
|
||
| const kbd = document.createElement("span"); | ||
| kbd.className = "kbd"; | ||
| kbd.textContent = "⌘⇧C"; |
Contributor
There was a problem hiding this comment.
P3: The shortcut hint only shows the Mac modifier even though the handler also supports Ctrl+Shift+C. Show both modifiers or choose by platform to avoid misleading Windows/Linux users.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/trace/trace-ui.ts, line 86:
<comment>The shortcut hint only shows the Mac modifier even though the handler also supports Ctrl+Shift+C. Show both modifiers or choose by platform to avoid misleading Windows/Linux users.</comment>
<file context>
@@ -0,0 +1,119 @@
+
+ const kbd = document.createElement("span");
+ kbd.className = "kbd";
+ kbd.textContent = "⌘⇧C";
+
+ panel.append(dot, status, grabButton, kbd, toggleButton);
</file context>
| } | ||
| @keyframes pulse { 0%, 100% { opacity: 1; } 50% { opacity: 0.35; } } | ||
| .status { opacity: 0.8; white-space: nowrap; } | ||
| button { |
Contributor
There was a problem hiding this comment.
P3: Restoring buttons with all: unset removes the browser focus indicator without adding a replacement. Add a :focus-visible style so keyboard users can see which trace control is active.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/trace/trace-ui.ts, line 42:
<comment>Restoring buttons with `all: unset` removes the browser focus indicator without adding a replacement. Add a `:focus-visible` style so keyboard users can see which trace control is active.</comment>
<file context>
@@ -0,0 +1,119 @@
+}
+@keyframes pulse { 0%, 100% { opacity: 1; } 50% { opacity: 0.35; } }
+.status { opacity: 0.8; white-space: nowrap; }
+button {
+ all: unset;
+ cursor: pointer;
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Consolidates the recent test-coverage work into one PR (previously #499, #500, #501, #502, #503, #505, #506, #507). Pure additive test coverage plus two small fixture/harness additions — no library source changes. Targets the gaps surfaced by the Playwright V8 coverage report, biased toward deep/guarded branches.
Unit tests (147 total)
freeze-updates.ts(top fragile-internals gap): dispatcher-patch replay order (store → transition → state), the 4-arm circular-chain merge + extraction, context freeze, and both try/catch recovery arms.next-server-frames.ts:enrichServerFrameLocationsowner-stack merge (early exits, first-match-wins, server-URL filtering) +<unknown>/null symbolication fallbacks.is-next-project-runtime.ts,get-next-base-path.ts(memoization viavi.resetModules).css-property-bounds.ts,parse-any-color.ts,css-baseline-measurement.ts.findTailwindClassacross all chip scales (spacing/border/z-index/opacity + guards).clamp-to-range,format-color-label,safe-decode-uri-componentcatch path, all 5 error classes, and expandedparse-package-name/parse-activation-key.E2E
ContextMenukey +Shift+F10context menu, inactive guard, and the window-refocus grace period.useStatehooks (useReducer/useTransition/useSyncExternalStore) via a dedicated fixture harness.Test plan
vp test run tests— 147 unit tests pass (20 files)freeze-hooks+keyboard-handlere2e — 10 pass on chromiumpnpm typecheckcleanpnpm lintcleanIndividual commits are preserved. Supersedes #499/#500/#501/#502/#503/#505/#506/#507.
Note
Medium Risk
Mostly additive tests and fixtures, but the bundled trace-mode feature (display capture, clipboard, downloads) is new production surface area without tests in this PR.
Overview
Adds a large test coverage sweep (~147 unit tests) across fragile internals and utilities, plus two Playwright specs and a small e2e fixture for freeze behavior beyond
useState.Unit tests exercise
freeze-updates(hook queues, dispatcher replay order, context masking, error recovery),enrichServerFrameLocationsand symbolication edge cases, Next runtime helpers, CSS/Tailwind parsing utilities, activation keys, package name resolution, and error classes.E2E mounts
FreezeHookHarnesson the Vite app (counters foruseReducer,useTransition,useSyncExternalStore, and context) and asserts values stay fixed in prompt/freeze mode then resume after unfreeze. Keyboard-handler specs cover ContextMenu / Shift+F10 and the post-refocus activation grace period.Also in this diff: a new
startTraceModeperformance-trace feature (screen replay buffer, jank/long-task detection, clip export) is exported from the package and exposed on the e2e appwindow— not covered by the new tests in this PR.Reviewed by Cursor Bugbot for commit ae051c9. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds a comprehensive unit and E2E coverage sweep for
react-grab, covering dispatcher/fiber freeze paths, keyboard handling, Next server frames, and utilities. Also addsstartTraceModefor performance capture and exposes it in the e2e app.Coverage
useStatehooks (useReducer,useTransition,useSyncExternalStore, context) via a new fixture harness.Shift+F10, inactive guard, and window-refocus grace period.enrichServerFrameLocationsand symbolication fallbacks when metadata is missing.New Features
startTraceMode(display capture, jank/long-task detection, keyframe/clip export) and exposed aswindow.startTraceModein the e2e app.Written for commit ae051c9. Summary will update on new commits.