Skip to content

fix(diffs): Preserve controlled editor selection - #973

Closed
jakeleventhal wants to merge 1 commit into
pierrecomputer:beta-1.3from
jakeleventhal:agent/preserve-editor-focus
Closed

fix(diffs): Preserve controlled editor selection#973
jakeleventhal wants to merge 1 commit into
pierrecomputer:beta-1.3from
jakeleventhal:agent/preserve-editor-focus

Conversation

@jakeleventhal

Copy link
Copy Markdown

What changed

  • Reuse the attached file instance when a controlled React parent echoes the
    editor's current name, language, and contents with a new cache key.
  • Add a React regression test covering both the controlled-value echo and a
    genuine external content replacement.

Why

useFileInstance previously passed every controlled file update to
instance.render. When an Editor.onChange consumer stored the returned file
and generated a new cache key, the adapter treated that echo as an external
document replacement. The editor rebuilt its document and dropped selection
and focus after each change.

The adapter now preserves editor-owned state for same-document echoes while
continuing to render real external changes normally.

Validation

  • moon run root:format root:lint
  • moonx diffs:typecheck
  • moonx diffs:test (1055 tests)

Edit a React File whose onChange value is stored by its parent: the
controlled value echo rebuilds the editor document and drops focus and
selection on every change.

Reuse the attached file instance when the echoed name, language, and
contents still match the editor document. Genuine external content
updates continue through the normal render path.
@vercel

vercel Bot commented Jul 12, 2026

Copy link
Copy Markdown

@jakeleventhal is attempting to deploy a commit to the Pierre Computer Company Team on Vercel.

A member of the Team first needs to authorize it.

@jakeleventhal
jakeleventhal marked this pull request as ready for review July 12, 2026 18:56

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5555dd4a0c

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const editorFile = contentEditable ? editor?.getFile() : undefined;
const renderFile =
editorFile !== undefined && hasSameEditorContents(editorFile, file)
? (instanceRef.current.file ?? file)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Render controlled echoes with fresh contents

When a controlled editor echoes an edit and this branch substitutes instanceRef.current.file, the object is still the pre-edit prop; the file renderer patches its render/line caches during editing but does not rewrite file.contents until cache cleanup. If a later React update changes a render option that forces a re-highlight (for example theme or tokenization options) while the controlled prop already contains the edited text, FileRenderer.renderFileWithHighlighter reads the stale instanceRef.current.file.contents and repaints the old text even though the editor document and prop are updated. Preserve the old identity/cache key without passing stale contents into forced renders.

Useful? React with 👍 / 👎.

): boolean {
return (
editorFile.name === file.name &&
editorFile.contents === file.contents &&

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid materializing editor text every render

In contentEditable mode this comparison runs from the layout effect on every React render, not just after controlled onChange echoes. editorFile.contents is the getter returned by Editor.getFile() and materializes the full TextDocument text, so any parent state change that re-renders a large editable file now allocates and scans the whole document even when the incoming file prop did not change. Gate this echo check on a changed incoming identity/cache key or track the last echoed value instead of reading the full document on every render.

Useful? React with 👍 / 👎.

const editorFile = contentEditable ? editor?.getFile() : undefined;
const renderFile =
editorFile !== undefined && hasSameEditorContents(editorFile, file)
? (instanceRef.current.file ?? file)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep echoed cache keys visible to onChange

When a controlled parent echoes an edit with a fresh cacheKey, this path renders the old instance file instead of the echoed prop, so the editor's internal #fileInfo.cacheKey never advances. On the next edit, Editor.getFile() still reports the original cache key to onChange; consumers that derive the next revision from the current key will keep emitting the same key after the first edit, even though the prop already moved to a newer revision. Preserve the document without rebuilding, but still update the editor's file identity for the echoed key.

Useful? React with 👍 / 👎.

@jakeleventhal

Copy link
Copy Markdown
Author

@SlexAxton @amadeus before i start spam addressing AI comments, can you please let me know what you think of direciton/intent?

@ije

ije commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

you can use the editor.getState() and editor.setState (or editor.setSelections) APIs to preserve selection. i'm currently working on a document cache API that holds all opened textDocument. (this should work on the vanillaJS components as well)

@ije

ije commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

see #976

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants