Repository navigation
test: golden-master interaction traces with a browser cross-check - #281
Merged
Merged
Conversation
Freeze callback order, event kinds, handle identity, DOM sizes and layout reads through real DraggableCore events. Moving fake layout and two render cadences expose north/west correction and stale-prop accumulation. Keep audited bugs as named goldens so fixes require intentional trace diffs. Type-check test support and run all golden cases in the normal suite.
Run the shared fixture against lib source and real Chromium layout so the fake layout model can be checked independently. Compare callbacks and DOM fields with the committed jsdom traces using trusted mouse input. Keep Playwright in a standalone browser project and support installed browser paths for offline use. Browser launch aborted in this sandbox, so runtime parity remains unverified.
The browser runner loaded Playwright from a nested package.json that nothing installed, so it could only run by hand. Move playwright@1.62.1 to a root devDependency (the version react-grid-layout pins) and run yarn golden:browser in CI after the unit tests. Locally: 42 of 42 browser traces match their jsdom golden files.
|
Important Review skippedToo many files! This PR contains 434 files, which is 134 over the limit of 300. To get a review, reduce the PR to 300 files or fewer by splitting it into smaller PRs or changing its base branch. Usage-priced reviews support at most 300 files.
 |
Packj audit found 1/1 risky dependencies.
|
| Registry | Package | Version | Risks |
|---|---|---|---|
| npm | react-draggable | 4.5.0 |
|
Sentinels ({$undefined}, {$number}) are plain objects, so a trace value could change to a lookalike without changing its golden file, and value.map turned array holes into null. Ordinary objects with a "$" key are now wrapped in {$escaped}, and holes encode as {$hole: true}. Same fix as react-grid-layout#2320 round 1. No golden file changed; a new serializer test covers each collision.
Packj audit found 1/1 risky dependencies.
|
| Registry | Package | Version | Risks |
|---|---|---|---|
| npm | react-draggable | 4.5.0 |
|
Review P2: the recorder kept only width and height from the box's style attribute, so a refactor that dropped a user's style (the fixture passes color: red) left every trace unchanged. Record all inline style properties, sorted. All 418 goldens regenerate with one style list each; nothing else moves. The browser cross-check failed on Linux CI while passing 42/42 locally, and the failing lines live only in the job log. On GitHub Actions the runner now also prints an ::error workflow command per failure, so the diff shows up as a check annotation.
Packj audit found 1/1 risky dependencies.
|
| Registry | Package | Version | Risks |
|---|---|---|---|
| npm | react-draggable | 4.5.0 |
|
Root cause: both runners queue requestAnimationFrame until the final animation-frame step, which also held react-draggable's scheduleRemoveUserSelectStyles cleanup. In browser-strict-box the s drag leaves a native Range selection (handle s to handle se) that covers the w handle. On Linux Chromium the w mousedown lands inside that selection and starts a native drag: dragstart/dragover/dragend replace mousemove/mouseup, so DraggableCore never sees the moves or the release, stays dragging, and later gestures resize through two handles at once (width 1890). Fix: drain queued frames, unrecorded and inside act, before each gesture (goldenBrowser.settle() in app.tsx, called from run.cjs; the same drain in harness.ts), as real frames would between separate drags. The final gesture's cleanup keeps its recorded animation-frame step. No golden files change. Verified: yarn golden:browser 42/42 in mcr.microsoft.com/playwright:v1.62.1-noble and on macOS; yarn test 502/502 with no golden diffs.
Packj audit found 1/1 risky dependencies.
|
| Registry | Package | Version | Risks |
|---|---|---|---|
| npm | react-draggable | 4.5.0 |
|
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
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.
Pins every observable output of
ResizableandResizableBoxbefore the perf and clarity refactors. No file underlib/changes.418 scenarios: both components, all 8 handles alone and together,
axis, min/max constraints,lockAspectRatio,transformScale0.5/1/2, handle as element and as function,draggableOpts, controlled sizes the parent does and does not update, mouse and touch, a final mouseup delta, unmount mid-resize, StrictMode. Two cadences: one render per event, and a whole gesture with no re-render (the #255 case).jsdom returns zero rects, so the w/n position correction never ran in tests before. A small layout shim computes rects from inline styles, which exercises it. The browser run checks the shim: 42 of 42 traces match real Chromium exactly. The
rectReadscounter is jsdom-only and is the one field left out of that comparison. It is pinned so the planned "skip the layout read for s/e handles" change shows up as an intended diff.Bugs are pinned, not fixed. Each has a scenario tagged
"knownBug":axis="x"still changes heightaxis="y"without width, lock with height 0onResizeafteronResizeStopdraggableOpts.onStart/onDrag/onStopare documented but overwrittenaxis="none"also suppresses onResizeStart/onResizeStopMutation check: breaking the w/n correction at
lib/Resizable.tsx:110,114fails 54 scenarios with readable diffs.playwright@1.62.1(the version react-grid-layout pins) becomes a devDependency, and CI runsyarn golden:browserafter the unit tests. Regenerate withyarn golden:update. Any later golden diff must be explained in its PR;__tests__/golden/README.mdhas the rules.Status - 2026-10-31
yarn testyarn golden:browserResizable.tsx100% on all fourReview gate:
codex-review.shwith__golden__/**excluded as generated (4.3 MB).Test plan