Repository navigation
fix(core): preload lazy editors to reduce React 19 first-edit latency - #1203
Open
jakelevirne wants to merge 1 commit into
Open
jakelevirne wants to merge 1 commit into
jakelevirne wants to merge 1 commit into
Conversation
…dit doesn't suspend Under React 19 the first type-to-edit after a page load suspended on the `React.lazy` overlay editor, and React held the Suspense reveal for up to 300 ms, so keys typed in that window restarted the edit and were lost (glideapps#1202). `preloadableLazy` keeps each editor in its own chunk but renders the loaded component directly once it has arrived. `DataEditor` preloads the overlay editor from a mount effect, and drawing a number cell preloads the number editor, so grids without number cells never fetch it. If an edit starts before the chunk arrives, the editor falls back to `React.lazy` as before. Each instance pins the component type it first rendered with, so a chunk that arrives while a transition render is suspended can't change the element type and remount an open editor. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This branch has not been deployed
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.
Summary
On React 19, the first type-to-edit after a page load opens the overlay editor about 300 ms late, and the keys typed in that window are lost. Typing
editedinto a Text cell savesdited. This PR starts loading both lazy editor chunks before editing, so an edit after the imports complete renders without suspending. Both editors stay in their own chunks; an edit started earlier still uses the lazy fallback.Cause
DataEditorrendersDataGridOverlayEditorthroughReact.lazyinside<React.Suspense fallback={null}>, andnumber-cell.tsxdoes the same withNumberOverlayEditor. Nothing loads the chunk before the first edit, so the first edit suspends. React 19 throttles when a suspended boundary's content is revealed: it holds it for up to 300 ms after the fallback appears, even if the chunk arrives within a few ms. React 18 revealed it as soon as the chunk resolved. While the editor is held back, each key reaches the grid and restarts the edit with itself as the initial value, so the first characters are dropped. After the relevant chunk resolves, subsequent edits are unaffected.Fix
A small internal helper,
preloadableLazy(src/common/preloadable-lazy.tsx, not exported), wrapsReact.lazy:preload()starts the import once. It returns the same promise on every call, and that promise never rejects. If the preload fails, the next render loads again and reports the error the wayReact.lazydoes.React.lazyand behaves as it does today.Where it's used:
DataEditorstarts loading the overlay editor in a mount effect. Once that import completes, opening the editor does not suspend; an earlier edit still uses the lazy fallback.preload()calls return the cached promise, so the per-draw cost is one function call.Why not a static import? The issue suggests one, and it would fix this too. But it would move both editors into every consumer's main bundle, including apps that never edit. Preloading keeps code splitting and makes the editor ready once its chunk arrives.
Transition hardening. Each instance chooses between the direct component and the
React.lazywrapper on its first render, and keeps that choice inuseState. Without that, on React 19.2 and later, an editor opened inside a transition before its chunk has loaded could be remounted, losing what was typed. In that case React suspends, yields, and when the module has arrived it re-renders only the lazy child, so the committed element type is the lazy wrapper. The next render of the parent would then return the loaded component, a different element type, and React would remount it. The grid's own edits (keydown, double-click, Enter) aren't transitions. But an app can open an editor from one, so the helper keeps the element type stable per instance.Tests
11 new core tests. Core goes from 387 to 398; cells (64) and source (7) are unchanged.
test/preloadable-lazy.test.tsx(8 tests): rendering after a preload doesn't suspend;preload()loads once, including under StrictMode's doubled effects; without a preload it suspends likeReact.lazy; a suspended mount isn't remounted after the module arrives, both in a normal render and in a transition; a failed preload doesn't reject, and a later render retries; a failed render-time load reaches the error boundary. The transition test runs on React's real scheduler instead ofact, becauseactnever yields, so it can't reproduce the replay.test/data-editor-first-edit.test.tsx(3 tests, in their own file because the editors' load state is module-level): a grid with no number cells doesn't load the number editor; the first keypress on a Text cell renders the overlay editor in the sameact; after number cells are drawn, the first keypress on a Number cell does the same.Without the fix (the unchanged
data-editor.tsxandnumber-cell.tsx), the Text and Number first-keypress tests fail on React 19.1.1 (the lockfile's version) and on 19.3.0. The transition test fails on 19.3.0 if the helper doesn't pin the element type (b/binstead ofa/b). On 19.1 and 18,React.lazydoesn't take the replay path, so that test passes either way.Browser evidence
This is a production Vite build of a small app: a Name Text column and an Age Number column, using the packed
@glideapps/glide-data-gridbuilt frommain(0875d78) and from this branch. For each of Chromium 153, Firefox 155 and WebKit 26.6, with 5 trials per cell kind:keydownto the editor's<input>/<textarea>getting focus.editedor12345at 75 ms per key, presses Enter, and reads back the saved value.Each trial waited for network idle before interacting. If an editor chunk is still in flight, the lazy fallback can still suspend; a deliberately delayed chunk reproduced that limit.
First-edit gap in ms (min–max over 5 trials) and the value saved by the first edit:
maindited×52345×5maindited×52345×5maindited×52345×5edited×512345×5edited×512345×5edited×512345×5mainedited×512345×5mainedited×512345×5mainedited×512345×5edited×512345×5edited×512345×5edited×512345×5Second edits on the same page saved the full value in every configuration. Opening an editor by double-click and by Enter also worked in every configuration.
Code splitting is kept. The Vite build still emits
data-grid-overlay-editor-*.js(3.76 kB) andnumber-overlay-editor-*.js(16.23 kB) as separate chunks, with the same sizes as onmain. The entry chunk grows by 0.33 kB. On a page that shows only the Text column, the number editor chunk is never requested; on a page with the Number column, it's requested before any edit.Checks
These are the CI steps, run locally on Node 20.10.0 on this branch:
npm install: no lockfile change.npm run build, including lint: passes, with the 2 existingno-consolewarnings inselection-serialization.stories.tsx.npm run test -- --coverage: passes, 398 tests.npm run test-source: passes, 7 tests.npm run test-cells: passes, 64 tests.npm run test-projects: passes.npm run test-19: passes, 398 tests on React 19.3.0.npm run test-18: passes, 398 tests. Note thatsetup-react-18-test.shinstallsreact@latest, which is 19.3.0 now, so I also installedreact@18andreact-dom@18by hand. On React 18.3.1, core (398), cells (64) and source (7) all pass.Compatibility
package.jsonor lockfile change.React.lazy,React.createElementanduseState, all available since React 16.8, so it stays within the supported range (16.12 through 19). I tested it on React 18.3.1, 19.1.1 and 19.3.0, but not on 16 or 17.The same fix, without the transition hardening, shipped in specstoryai/ai-data-grid#59.
Fixes #1202
🤖 Generated with Claude Code