fix(codemirror): stop debounced edits from being dropped/reverted - #10406
Open
kwburns-kong wants to merge 4 commits into
Open
fix(codemirror): stop debounced edits from being dropped/reverted#10406kwburns-kong wants to merge 4 commits into
kwburns-kong wants to merge 4 commits into
Conversation
…ale-write races one-line-editor.tsx fired two independent onChange triggers per edit: an immediate blur-flush and a separately-scheduled 100ms-debounced call from the CodeMirror 'changes' event. Blur never cancelled the pending debounced timer, so if another edit (e.g. adding a new environment KV row) landed within that 100ms window, the stale debounced call fired afterward with a closure over the pre-edit state and silently reverted/re-persisted it - the root cause of the flaky "manage environment" smoke test (row added via Add Row would vanish, environment values would fall back to the base environment). misc.debounce() now exposes cancel(), and the blur handler calls it before flushing. Also retains the persistedPairs/commitPairs local-state fix in key-value-editor.tsx (avoids building the next array from a stale `data` prop snapshot) - both fixes were needed to fully close the race. Verified with 8 consecutive full runs of environment-editor-interactions.test.ts (previously ~50% flake rate, now 0/8 failures) and a clean full npm test run (149 files, 2428 passed).
…-render OneLineEditor's debounced change/blur listeners depended on onChange and updateTooltipValue by reference, so any parent re-render (a fresh inline onChange closure, or handleRender's loader data getting a new reference after a fetcher revalidation) tore down the listener and cancelled a pending debounced call before it could fire, silently dropping the edit. Track both in refs so the listeners are stable across re-renders. Also close a related hint-dropdown leak: completeAfter() only checked focus before its async lookups, not after, so a slow-resolving autocomplete could pop a hint over an editor the user had already left, and since it was orphaned it never got a blur event to close it.
✅ Circular References ReportGenerated at: 2026-08-17T00:04:36.361Z Summary
Click to view all circular references in PR (10)Click to view all circular references in base branch (10)Analysis✅ No Change: This PR does not introduce or remove any circular references. This report was generated automatically by comparing against the |
…ocal edits EnvironmentKVEditor's persistedPairs resync effect overwrote local optimistic state on every `data` prop change with no ordering check. Because environment updates round-trip through an async fetcher/loader that doesn't resolve in submission order, a slower earlier write's revalidation (e.g. Delete All) could land after a faster later edit and silently clobber it - reintroducing deleted rows or dropping a value that had already committed. Thread the owning environment's `modified` timestamp through as a `dataRevision` prop and only accept a resync when it's at least as new as the last one applied, so an out-of-order/stale delivery is ignored instead of overwriting fresher local state.
…sable Row The Disable Row edit persists asynchronously through the same fetcher that gates the Close button (isDisabled while a change is in-flight), but this test clicked Close immediately after asserting the row's opacity instead of waiting for that persistence to round-trip first - unlike every other edit->action transition in this file, which does wait. On a slow/contended CI runner this let Close be clicked while still disabled, a no-op click that left the Manage Environments dialog open and the next waitFor hanging for the full 30s timeout.
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.
What this PR does
Cherry-picks the CodeMirror
OneLineEditordebounce flake fix (+ flake hardening) from #10302.misc.debounce()now exposescancel(), andOneLineEditor's blur handler calls it before flushing — previously a blur-flush and a still-pending 100ms debouncedchangescall could both fire, with the stale debounced call clobbering state with a pre-edit snapshot. This was the root cause of the flaky "manage environment" smoke test (a row added via Add Row would vanish, or values would fall back to the base environment).EnvironmentKVEditor.handleDeleteItemnow goes throughcommitPairsinstead of callingonChangedirectly, for the same local-state-consistency reason.OneLineEditor'schanges/blurlistener effects now trackonChange/updateTooltipValuevia refs instead of depending on their identity, so a parent re-render (e.g. a fresh inlineonChangeclosure, or a fetcher revalidation that changesupdateTooltipValue's reference) can no longer tear down the listener and cancel a pending debounced call before it fires.autocomplete.ts'scompleteAfter()now re-checks focus after its async lookups, so a slow-resolving autocomplete can't pop a hint over an editor the user has already left (that hint would otherwise be orphaned, sincecloseOnUnfocusonly starts listening once the widget exists).Test plan
npm run type-check(packages/insomnia) — cleannpx eslinton all touched files — cleannpm test -w packages/insomnia --silent— 161 files / 2285 tests passed, 0 failurespackages/insomnia-smoke-test/tests/smoke/environment-editor-interactions.test.tsrun 50 consecutive times — 50/50 passed, 0 failures