Skip to content

upstream(@pierre/diffs): renderDiff keeps highlighted cache when content changes, stale paints on restore #1208

Description

@backnotprop

TLDR: @pierre/diffs@1.3.2 DiffHunksRenderer.renderDiff refuses to swap its render cache for new diff content while the cache is highlighted. On the worker-pool path, a repaint after updateItem with a new fileDiff (fresh cacheKey) keeps painting the stale cached content and only queues an async worker highlight of the new content. If that worker task never lands (for example, it gets invalidated by a theme sync calling invalidateRenderTasks), the stale content stays on screen permanently. We hit this in edit mode: after Discard, the pristine restore write landed in item state but the diff kept displaying the edited buffer (#1193 QA finding). We work around it in writeRestore by calling clearRenderCache() plus rerender() on the live instance (PR linked below). This issue tracks reporting or patching it upstream.

Where

dist/renderers/DiffHunksRenderer.js, renderDiff, in the worker-pool branch (approximately lines 431-441 of the 1.3.2 dist):

const newContent = !areDiffTargetsEqual(diff, this.renderCache.diff);
...
if (!highlightPending && (forcePlainText || this.renderCache.result == null || !this.renderCache.highlighted && (newContent || newRenderRange))) {
  this.renderCache.diff = diff;
  ...
}
if (!forcePlainText && hasContent && (!this.renderCache.highlighted || forceHighlight)) this.workerManager.highlightDiffAST(this, diff);

The swap condition only honors newContent when !this.renderCache.highlighted. When the cache IS highlighted and has a result, new content does not replace renderCache.result synchronously; the renderer paints the old cached rows and relies entirely on the async highlightDiffAST round trip to heal.

How we hit it

  1. An edit session ends (upstream's documented commit pattern: one combined updateItem carrying edit: false plus the restored fileDiff with a fresh cacheKey).
  2. The session leaves renderCache.highlighted === true (session rendering runs locally with the token transformer forced on).
  3. The teardown repaint takes the branch above: newContent is true, but the highlighted cache blocks the swap, so the stale edited rows keep painting.
  4. Pristine pixels arrive only if the queued worker highlight completes, which takes 30ms to seconds, and never arrives at all when the task is invalidated first (we observed this with a concurrent setRenderOptions theme sync).

Reproduced empirically with a real CodeView plus worker pool under happy-dom: after the combined restore write, the DOM still contained the edited buffer at t+1000ms in the invalidation case. Our regression test for the workaround is packages/review-editor/edit/discardRestoreRender.test.tsx.

Suggested upstream fix

In the worker branch, let newContent force the cold path even when the cache is highlighted: swap renderCache.diff, drop highlighted to false, and rebuild the plain AST result synchronously (the same UX as any diff switch: plaintext first, async highlight after). Alternatively, treat a highlighted cache with newContent the same as renderCache.result == null.

Workaround on our side

writeRestore in packages/review-editor/edit/useEditSession.ts clears the live instance's render cache and calls rerender() after the restore updateItem, forcing the next paint down the cold-render path. This reaches into the protected hunksRenderer and should be removed once upstream honors newContent for highlighted caches.


This issue was drafted with AI assistance; the root cause was established with an empirical repro harness against the unpatched dist.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions