Skip to content

Conversation

@wu-json
Copy link
Contributor

@wu-json wu-json commented Sep 30, 2025

Summary

Implements incremental rendering by only re-writing lines in the output that have changed.

Problem

The current render implementation erases and re-draws all content on each turn. This can result in flickering in some applications.

There have been other attempts at solving this issue (#751, #413, #708) but they seem to be abandoned.

Impact

Before Incremental Rendering

Notice the stutter in the beginning as the processes startup.

Screen.Recording.2025-09-29.at.9.41.57.PM.mov

After Incremental Rendering

Screen.Recording.2025-09-29.at.9.42.51.PM.mov
@wu-json wu-json force-pushed the feat/incremental-rendering branch from a6ca093 to a427701 Compare September 30, 2025 04:33
@wu-json wu-json changed the title Feat/incremental rendering Sep 30, 2025
@wu-json wu-json marked this pull request as ready for review September 30, 2025 04:56
@wu-json wu-json force-pushed the feat/incremental-rendering branch from a427701 to d71d019 Compare September 30, 2025 05:35
@blakemckeany
Copy link

Hello,

I'm also having this same flickering problem, however I tested your changes and am still having no luck with stopping the flicker (quite severe in this example)

flickering.mp4

It seems to become a problem when

a) I'm rendering boxes, b) the box height is > 15ish.

Here is the same example with 15 box height..........

noflickering.mp4

Any ideas? Much appreciated :)

@wu-json
Copy link
Contributor Author

wu-json commented Oct 1, 2025

Hello,

I'm also having this same flickering problem, however I tested your changes and am still having no luck with stopping the flicker (quite severe in this example)
flickering.mp4

It seems to become a problem when

a) I'm rendering boxes, b) the box height is > 15ish.

Here is the same example with 15 box height..........
noflickering.mp4

Any ideas? Much appreciated :)

Hey @blakemckeany! The flickering still being present in your example makes sense. This PR uses a line-based diff to determine which lines to re-render. However in your example, since the colors are changing across columns, this PR (and all of the ones before me) won't prevent flickering because every line will have a line change due to the color and thus get wiped and re-rendered.

We could look at a character-based diffing strategy in the longer term to handle these cases but that does add a considerable amount of complexity.

I view this PR as an incremental step towards that. Think we can get this merged first and I could look at something more inclusive of all cases as a followup.

@blakemckeany
Copy link

Hello,
I'm also having this same flickering problem, however I tested your changes and am still having no luck with stopping the flicker (quite severe in this example)
flickering.mp4
It seems to become a problem when
a) I'm rendering boxes, b) the box height is > 15ish.
Here is the same example with 15 box height..........
noflickering.mp4
Any ideas? Much appreciated :)

Hey @blakemckeany! The flickering still being present in your example makes sense. This PR uses a line-based diff to determine which lines to re-render. However in your example, since the colors are changing across columns, this PR (and all of the ones before me) won't prevent flickering because every line will have a line change due to the color and thus get wiped and re-rendered.

We could look at a character-based diffing strategy in the longer term to handle these cases but that does add a considerable amount of complexity.

I view this PR as an incremental step towards that. Think we can get this merged first and I could look at something more inclusive of all cases as a followup.

Hi @wu-json this makes a lot of sense! Thanks for taking the time to look into it and agree that this is a good first step towards the larger goal. I'll continue to look into this as well and help out where I can

@wu-json
Copy link
Contributor Author

wu-json commented Oct 14, 2025

@sindresorhus Anything else I need to do here to get this in?

@sindresorhus
Copy link
Collaborator

If you run npm run example examples/use-input and press right-arrow key, it doesn't clear the old frame.

@sindresorhus
Copy link
Collaborator

I'd like to see a few more tests:

  • Shrinking output keeps screen tight
    Render three lines, then two, then one. Assert the third render’s write buffer matches what a straight erase-and-redraw would produce (no dangling cursorNextLine or blank row at the bottom). That catches off-by-one errors when erasing extra lines.
  • clear() fully resets incremental state
    Render multiple lines, call render.clear(), then render a new single-line frame. Verify the very next write is exactly a single eraseLines(...) + the fresh line, with no leftover cursor moves from the old frame.
  • done() resets before next render
    Render once, call render.done(), then render again. Check that the first write after done() is a clean full redraw (matching what a brand-new logUpdate instance would output). This ensures the cursor bookkeeping does not leak across lifecycles.
@wu-json
Copy link
Contributor Author

wu-json commented Oct 15, 2025

@sindresorhus Ready for a second scan.

@sindresorhus
Copy link
Collaborator

AI review:


  1. Dead branch. Inside the render loop you check if (i === lineCount - 1) but the loop is i < lineCount - 1, so that branch is unreachable. Delete it.

  2. Cursor clear math: explain it or name it. The shrink path uses
    eraseLines(previousLineCount - lineCount + 1) + cursorUp(lineCount - 1). It’s correct (see tests), but non-obvious. Either extract to well-named helpers or add a short comment that this clears trailing lines plus the final newline slot.

  3. Remove redundant nullish coalescing. lines[i] ?? '' is unnecessary because i < lineCount - 1 guarantees lines[i] exists.

Tests: coverage is good; add a couple of edge cases

  • Multiple consecutive clear() calls (should be harmless no-ops).
  • sync() followed by update (assert incremental path is used).
  • Render to empty string (full clear vs early exit).
    You already assert “single write call with multiple surgical updates” and “shrinking output keeps screen tight” (good).

Small refactors (readability, fewer footguns)

  • Hoist computed counts:

    const prevCount = previousLines.length;
    const nextLines = (str + '\n').split('\n');
    const nextCount = nextLines.length;
    const visible = nextCount - 1;

    Then use prevCount, nextCount, visible in math and loop bounds. It makes the off-by-one logic obvious and lets you delete the dead branch.

  • Drop commitBuffer and just stream.write(buffer.join('')); at the end.

@wu-json
Copy link
Contributor Author

wu-json commented Oct 17, 2025

@sindresorhus Just addressed all of the review comments above.

@sindresorhus
Copy link
Collaborator

I'm not confident in the changes, so I think it should be made opt-in for now through a incrementalRendering option. You can just duplicate create function to keep the old logic.

@wu-json wu-json requested a review from sindresorhus October 27, 2025 08:52
@wu-json
Copy link
Contributor Author

wu-json commented Oct 27, 2025

@sindresorhus Ready for another review.

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

Labels

None yet

3 participants