Repository navigation
v0.28.4 — starting and finishing a connection responds faster - #359
Merged
Merged
Conversation
…ace handles once Share note 58. While a connection or an end grip is dragged every node mounts its face handles, and at the end it unmounts the unused ones — every node, in the same commit. Each node then measured its handles on its own: one React Flow `updateNodeInternals` call per node, each a store update run through every subscriber (the 63-node gacha banner Template: 848 handles). Counted on main: 61-62 measurement calls per start and as many per-node frames per release. Now the first node whose layout effect sees the drag state change measures every node in ONE call (`src/components/nodes/faceBatch.ts`), and the others skip: - at the start, at once: React applies the whole commit's DOM before any layout effect runs, so every node's new handles are in the DOM, nothing is painted, and React Flow's edges have not caught up — the moment each node measured itself in before; - at the end, on the next animation frame (only removals), as one node's removal is (the React Flow 008 order of #356 is kept); - a new drag cancels an end's measurement still waiting. Every other mount change (selection, focus, a document change) is measured per node as before. `docs/edge-routing.md` §ER17.1 states the rules. This is option 1 of the four arms measured (main / 1 / 2 / 1 + 2); the code is the measured copy with the batch moved into its own module. Production build, two interleaved rounds, medians of eight drags (input event to the second animation frame): gacha start 113-117 -> 73-74 ms and release 123-132 -> 78-83 ms at CPU x1; 795-817 -> 371-377 ms and 674-712 -> 280-301 ms at x4; Coffee not slower. Measurement calls per start / release 61-62 -> 1; the store updates in those windows 64-65 -> 4 and 75-81 -> 15. Tests: - `faceBatch.test.ts` (6): one commit's requests are one measurement of every node, at once for a start, on the next frame for an end; a new drag cancels the waiting end; the nodes are the canvas's when it runs. Two deliberate mutations (no merging; no cancel) fail it. - `e2e/connection-draw-faces.spec.ts` (5): one drag can land on an unused face of every node (React Flow picks each from its measured bounds — a node left out of the measurement fails it, checked by mutation); a drag released and started again in one task, and five back to back, then a drop on a face; a refused drawing then the next connection; Esc on an end grip drag then the next connection; no 008 in the console throughout. Checks: tsc, lint, full vitest (3 598 passed, 1 skipped), e2e shards; dev, fresh server: the new spec and the 9 related specs, 109 passed.
… faster - package.json / package-lock.json 0.28.4; `.changes/draw-start-batch.json` (user-facing, `release:0.28.4`, Lumi's classification 2026-10-17), date 2026-10-17, to be checked on the merge day. - What's new 0.28.4, three lines in 18 languages (Daisy's draft on Lumi's direction "the response when you start and finish drawing a connection is improved", for review; the measured reductions stay in the CHANGELOG and the technical record): starting to draw responds faster, especially in large diagrams; letting go too, made or dropped on the empty canvas, and dragging an end grip; nothing else changes, every side of every node is still offered. The release-note validator asks for 3 to 5 lines, so the one-sentence direction is three lines. Grip / canvas / side words follow each locale's 0.27.0 line and UI. es-ES = es-419 and pt-PT = pt-BR, written neutrally; French avoids the punctuation its spacing rule governs. Locale count pins follow (catalog 1148, runtime 1370); ru adds `создаётся` to its ё list; the e2e release list pins 0.28.4. - CHANGELOG v0.28.4 (what was slow, now, how much on one machine); README Latest v0.28.4 (v0.28.3 kept as a section, v0.28.0 moved into the CHANGELOG list). The translation contract tests pass: evidence of the catalogs' structure and terms, not a review of how the lines read; no native-speaker review. Checks: tsc, lint, full vitest (3 598 passed, 1 skipped), change declaration, i18n, i18n surface, control bytes, isolate / ICU arguments, locale lists, functional glyphs, e2e shards, snapshot policy, form / content direction, no pseudo-locales.
Deploying cozy-loop-studio with
|
| Latest commit: |
6380755
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://af29d737.cozy-loop-studio.pages.dev |
| Branch Preview URL: | https://perf-draw-start-batch.cozy-loop-studio.pages.dev |
…cked (Lumi's review) Lumi's review of the report and the measurement record (2026-10-17; not a code review of the product diff): - the second line said that a release which makes a connection, and an end grip drag, got faster; only a drag released on the empty canvas was timed (the others were checked to work, not timed), so it now says that one; - "Nothing else changes" was wider than what was checked; the third line is now the checked contract only: while drawing a connection, handles on every side of every node remain available. English and Korean in Lumi's words; the other 16 follow (the port / point word each locale's UI uses for a handle; pt says "arrastada" for both regions, the pt-PT audit refusing the estar + gerund form). README Latest and the CHANGELOG follow; the CHANGELOG says which gestures were timed. ru drops `создаётся` from its ё list again (the word left the line). No native-speaker review. Checks: tsc, lint, full vitest (3 598 passed, 1 skipped), change declaration, i18n, i18n surface, control bytes, isolate / ICU arguments, locale lists, functional glyphs; whats-new.spec.ts on a fresh dev server, 96 passed. The full local e2e is not rerun for this wording-only change (Lumi).
… ends when React Flow's connection ends
Found in my own review of the diff, on the points Lumi named for the code
review (state per canvas; a waiting measurement on restart, document
replacement and unmount). `faceBatch.ts` kept one module-wide flag "this
drag is measured", turned on at a start and off at an end reported by a
node. If no node saw the end — every node gone during the drag (its insert
undone with Ctrl+Z, or the node structure switched) — the flag stayed on,
and the NEXT drag's start was taken for "already measured": no node's faces
were measured, so React Flow could not find an unused face to drop on.
Reproduced before the fix in a unit test and in an e2e (a module inserted
into an empty document, a drag from one of its nodes, Ctrl+Z while held,
release, Ctrl+Y, a new drag: the target's unused face never gets
`connectingto`).
Now:
- a drag also ends when React Flow's `connection.inProgress` goes false (a
store subscription, one per canvas), so the next start is measured even
when no node saw the end; the end seen by the store and by the nodes is
one measurement;
- the state is each canvas's, keyed by the store's own `getState`.
`useStoreApi()` gives every component its own `{ getState, setState,
subscribe }` wrapper (memoized per hook call): keyed by the wrapper, every
node had its own state and nothing was merged — a first attempt keyed that
way measured all 63 gacha nodes 61 times per start (counted) and made a
start about 3 times and a release about 6 times slower than main
(production, x1); never committed;
- unchanged: a start measures every node at once, an end on the next
frame, a start cancels an end's frame that has not run. A frame is not
cancelled when the canvas goes away; it runs, reads the store and the DOM
then, finds no `domNode` (React Flow resets its store on unmount) and
measures nothing.
Counted again (dev, diagnostic copies, never in a timing run): one
measurement of every node per start and per release, as before the fix.
Tests:
- `faceBatch.test.ts` 11 (was 6): + requests of one start in separate
tasks are still one measurement; + an end seen by React Flow and by the
nodes is one measurement; + a drag whose end no node reported is over
when React Flow ends it; + every node's own wrapper of one store is one
canvas; + two canvases are measured apart; + an end with no drag begun
measures nothing. The test-only reset is gone (each test makes its own
store). The module-wide version fails 6 of them, the per-task (microtask)
version 3, the wrapper-keyed version 1.
- `e2e/connection-draw-faces.spec.ts` 6 (was 5): + every node gone during a
drag (its insert undone) does not stop the next drag measuring every
node; it fails on the previous faceBatch and passes now. After the redo
a click on the empty pane did not clear the restored selection there (not
traced, left open), so the test clears it through the store;
`emptySpot` now looks for a point where the pane is on top.
- `docs/edge-routing.md` §ER17.1 says how a drag ends and what the state is
keyed by.
Production timing of this code against main (base twice, this twice,
interleaved, medians of eight drags): gacha start 86.8-92.2 -> 62.8-62.9 ms
and release 85.3-87.6 -> 61.6-64.4 ms at CPU x1; 502.7-506.0 -> 285.6-291.1
ms and 438.4-471.3 -> 213.4-228.1 ms at x4; Coffee not slower.
Checks: tsc, lint, full vitest (3 603 passed, 1 skipped), e2e shards,
control bytes; dev, fresh server: the new spec and the 9 related specs,
110 passed.
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.
v0.28.4 — starting and finishing a connection responds faster
While a connection or an end grip is dragged every node mounts its face handles, so the drop can choose a side, and when the drag ends it unmounts the unused ones — every node, in the same commit. Each node then measured its handles on its own: one React Flow
updateNodeInternalscall per node, each a store update run through every subscriber (the 63-node gacha banner Template has 848 handles mid-drag). Share note 58 holds the investigation, the four measured arms (main / batch the measurement / re-render only the handles / both) and Lumi's corrections to the method. Four commits: the change, the release, the wording after Lumi's review of the report and the measurement record, and a fix found in the code review (section 1b).1. The change (
66ad42b,src/components/nodes/faceBatch.ts,nodes.tsx)updateNodeInternalscall; the others skip.docs/edge-routing.md§ER17.1 states the rules.1b. The fix from the code review (
6380755)connection.inProgressgoes false (a store subscription, one per canvas); the end seen by the store and by the nodes is one measurement.getState.useStoreApi()gives every component its own{ getState, setState, subscribe }wrapper, so a key on the wrapper gives every node its own state: a first attempt keyed that way merged nothing (counted: all 63 gacha nodes measured 61 times per start) and was about 3 times slower at a start and 6 times at a release than main; it was never committed, and a unit test now pins that the wrappers of one store are one canvas.domNode(React Flow resets its store on unmount) and measures nothing.2. Measured (share note 58; production build, one machine, medians of eight drags, input event to the second animation-frame callback — a comparison measure, not the paint)
Ranges over the arm runs and the committed-code runs (four sittings in all). The fixed code (
6380755) was timed once more against main in a fifth, faster sitting: gacha start 86.8–92.2 → 62.8–62.9 ms and release 85.3–87.6 → 61.6–64.4 ms at CPU × 1, 502.7–506.0 → 285.6–291.1 ms and 438.4–471.3 → 213.4–228.1 ms at × 4; Coffee not slower. Coffee roastery operations flow: not slower in any row. Counted in a separate diagnostic run (counters in scratch copies, never in a timing run): measurement calls per start / release 61–62 → 1; React Flow store updates in those windows 64–65 → 4 and 75–81 → 15 (React Flow's own updates remain). Face handles mid-drag 636 / 189 and after the release 80 / 13, as on main.3. Tests
src/components/nodes/faceBatch.test.ts(11): the requests of a drag's start are ONE measurement holding every node on the canvas, at once, also when they come in separate tasks; an end is one measurement on the next frame, whether React Flow or the nodes report it; a new drag cancels a waiting end; a drag whose end no node reported is over when React Flow ends it; the wrappers of one store are one canvas; two canvases apart; the nodes are the canvas's when it runs. It pins the measurement calls, not React Flow's store update count. The module-wide first version fails 6 of them, a per-task variant 3, the wrapper-keyed variant 1.e2e/connection-draw-faces.spec.ts(6), dev build (008 prints only there): one drag lands on an unused face of every node (React Flow'sconnectingtocomes from its measured bounds; leaving one node out of the measurement → 4 of 5 fail); a drag released and started again in one task (before the end's frame) and five drags back to back, then a drop on a face — made on that face, ending on its handle, its line in the DOM on every frame, the faces back afterwards; a refused drawing (a repeat of a connection drawn the same way) then the next connection; Esc on an end grip drag then the next connection; every node gone during a drag (a module insert undone while dragging, then redone) and the next drag still measures every node (fails on the first version); no 008 in the console. In that last test a click on the empty pane after the redo did not clear the restored selection — not traced, left open — so the test clears it through the store.4. Release (
c287eb3, wordinga792442).changes/draw-start-batch.json(user-facing), CHANGELOG v0.28.4, README Latest. Release date 2026-10-17, to be checked on the merge day.Verification, by head
63807556380755check:e2e-shards,check:control-bytes; the new spec and the 9 related specs, dev, fresh server; production timing against maina792442a792442check:change-declaration,check:i18n,check:i18n-surface,check:control-bytes,check:isolate-arguments,check:icu-arguments,check:locale-lists,check:functional-glyphs;whats-new.spec.tson a fresh dev serverc287eb3c287eb3--workers=1 --retries=0(chromium, mobile, portable)c287eb3distconfig, thenpwaconfig, run separatelyc287eb3check:change-declaration,check:i18n,check:i18n-surface,check:control-bytes,check:isolate-arguments,check:icu-arguments,check:locale-lists,check:functional-glyphs,check:e2e-shards,check:snapshot-policy,check:form-direction,check:content-direction,check:no-pseudo-locales66ad42bThe wording commit changes only release-note strings, README and the CHANGELOG; the full local e2e was not rerun for it.
Not covered
A phone (the diagram is read-only there, so no drag reaches this path; the mobile project ran in the full e2e), MMO, a real GPU display, the moment of paint.