Skip to content

Prevent repeated preview re-renders with async Shiki highlighting#111

Closed
tnhu wants to merge 3 commits intopanphora:mainfrom
tnhu:fix-shikijs-rerender-loop
Closed

Prevent repeated preview re-renders with async Shiki highlighting#111
tnhu wants to merge 3 commits intopanphora:mainfrom
tnhu:fix-shikijs-rerender-loop

Conversation

@tnhu
Copy link
Copy Markdown
Contributor

@tnhu tnhu commented Apr 12, 2026

Summary by cubic

Fixes the shiki syntax highlighting re-render loop by making onChange fire only on real content changes and updating docs/examples to use guarded, cached async highlighting. Editors stay stable while highlights load; distribution files are restored.

  • Bug Fixes

    • Centralized _notifyChange(); onChange now fires on input and when setValue actually changes content, not during render; safe to call updatePreview() inside onChange.
    • Shiki docs/example use full-code cache keys with a pendingHighlights guard; switched to editor.setCodeHighlighter and editor.updatePreview() to avoid feedback loops.
    • Added tests for single-fire on setValue, ignoring render-only updates, and non-recursive preview refresh.
  • Refactors

    • Restored dist/* for distribution.
    • README updated to reflect ~117KB size.

Written for commit 9035858. Summary will update on new commits.

@tnhu tnhu changed the title Fix Shikijs keeps re-rendering Prevent repeated preview re-renders with async Shiki highlighting Apr 12, 2026
panphora pushed a commit that referenced this pull request May 2, 2026
)

Async highlighters like Shiki were causing an infinite re-render loop:
onChange fired on every updatePreview() call, including renders triggered
by the highlighter resolving — which re-entered the cycle.

Centralize change notification behind _notifyChange() and only fire it on
real content changes (input events and setValue() with a different value),
not on every render. Update the Shiki docs/example to use a pendingHighlights
guard and editor.updatePreview() instead of OverType.setCodeHighlighter()
to refresh, removing the second source of recursion.

Adds tests for single-fire on setValue, ignoring render-only updates, and
non-recursive preview refresh.

Co-authored-by: Tan Nhu <tannhu@gmail.com>
@panphora
Copy link
Copy Markdown
Owner

panphora commented May 2, 2026

Merged in 35d93fd via cherry-pick (rebuilt dist/ locally rather than carrying it in the merge). Original commits and authorship by @tnhu preserved on the resulting commit.

Thanks for the fix — this is a real footgun for anyone wiring up Shiki, and the change to _notifyChange() is the right place for it.

Will ship in the next release.

@panphora panphora closed this May 2, 2026
panphora added a commit that referenced this pull request May 2, 2026
Adds new entries for be5invis (#108), Danny Vink (#109), yurivish (#110),
phinnaeus (#110 diagnosis), and Tan Nhu (#111). Appends new reports to
existing entries for 1951FDG (#102) and nodesocket (#101).
@panphora
Copy link
Copy Markdown
Owner

panphora commented May 2, 2026

Shipped in v2.3.7 (commit 35d93fd).

Centralized change notification behind _notifyChange() and only fire it on real content changes (input events and setValue() with a different value), not on every updatePreview(). Updated the Shiki docs example to use a pendingHighlights guard and editor.updatePreview() for the refresh, eliminating both sources of recursion.

Thanks @tnhu for the fix and the test cases.

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