[diffs/edit] fix editor state persisting - #1048
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de9a0cad7a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const persistedCacheKey = | ||
| this.#options.persistState === true | ||
| ? requirePersistedCacheKey(fileOrDiff) | ||
| : undefined; |
There was a problem hiding this comment.
Avoid reusing cached documents for recreated diffs
When persistState is true for a FileDiff, this now derives a persisted cache key and allows #getCachedTextDocument to win for diffs as well as files. If a React surface recreates FileDiffMetadata from the original before/after after an edit (same cacheKey) instead of reusing the mutated metadata object, FileDiff.attachEditor has no __prepareFile-style rerender path, so the editor's TextDocument/API and the already-rendered diff DOM diverge and the next edit applies against hidden cached text. Restrict cached-document reuse to File or add a diff prepare/rerender path.
Useful? React with 👍 / 👎.
| // scrollTop is deliberately not part of this staleness check: the | ||
| // surface can legitimately adjust vertical scroll while an async read | ||
| // is in flight (height reconciliation, clamping), and that must not | ||
| // block the restore. |
There was a problem hiding this comment.
Keep async restores from overriding user scroll
With async persistStateStorage (including indexedDB), a user can scroll the editor before get(cacheKey) resolves. Because the stale-restore guard compares scrollLeft but deliberately ignores the newly persisted scrollTop, the delayed restore can still call #setViewportScrollTop and yank the viewport back to the old stored position; please either include vertical scroll in the staleness signal after layout reconciliation or distinguish internal layout adjustments from user scrolls.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Added user scroll detection during async state restoration in 6558158
add
sharedEditorprop forEditProviderfix state persisting ignore scroll top position with
persistStateoption set totrue.Screen.Recording.2026-08-04.at.01.37.51.mov
cache editor instances by
editorOptionsre-structure edit docs page