Skip to content

[diffs/edit] fix newline inserts without virtualizer - #1066

Closed
ije wants to merge 1 commit into
mainfrom
ije/fix-file-edting
Closed

[diffs/edit] fix newline inserts without virtualizer#1066
ije wants to merge 1 commit into
mainfrom
ije/fix-file-edting

Conversation

@ije

@ije ije commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

this bug was introduced by #1057

Screen.Recording.2026-08-08.at.22.34.22.mov

@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
pierre-docs-diffs Ready Ready Preview Aug 8, 2026 2:35pm
pierre-docs-diffshub Ready Ready Preview Aug 8, 2026 2:35pm
pierre-docs-trees Ready Ready Preview Aug 8, 2026 2:35pm
pierrejs-diff-demo Ready Ready Preview Aug 8, 2026 2:35pm

Request Review

@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: 748809bcd5

ℹ️ 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 dirtyLines = tokenizer.tokenize(
change,
renderRange,
!this.#isDiff && fileInstance.getEditorViewport !== undefined

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 Don’t use the viewport hook as a row-realignment signal

This predicate treats any non-diff component that defines getEditorViewport as a host that will realign shifted rows, but that hook is also valid on a plain File just to bound editor focus/scrolling (see packages/diffs/test/editorPublicApi.test.ts:125-130). In that setup, inserting a newline after cached grammar states takes the shifted-state fast path again, so tokenize() can settle after only the split lines while File.applyDocumentChange() does not rerender/realign the existing DOM rows; the following row can still be left missing or stale, which is the same failure this patch is trying to avoid. Use a capability tied to virtualization/row realignment rather than the viewport callback.

Useful? React with 👍 / 👎.

@ije ije closed this Aug 9, 2026
ije referenced this pull request Aug 9, 2026
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.

1 participant