Skip to content

Fix progress bars not clearing on completion after React 19 upgrade#7062

Open
nickwesselman wants to merge 2 commits intomainfrom
fix/progress-bar-clearing-react19
Open

Fix progress bars not clearing on completion after React 19 upgrade#7062
nickwesselman wants to merge 2 commits intomainfrom
fix/progress-bar-clearing-react19

Conversation

@nickwesselman
Copy link
Contributor

@nickwesselman nickwesselman commented Mar 20, 2026

Summary

  • The Ink 6 / React 19 upgrade (Upgrade Ink to v6 and React to v19 #6831, 269f3aa6) deferred unmountInk() in ConcurrentOutput with setImmediate() to let React 19 flush batched state updates, but missed the same pattern in useAsyncAndUnmount (used by Tasks) and SingleTask
  • Without the deferral, unmountInk() fires before the setState that triggers return null is flushed, so the final render still contains the LoadingBar and it is never erased from the terminal
  • Wraps unmountInk() in setImmediate() in both places, matching the existing fix in ConcurrentOutput

Repro: shopify kitchen-sink async — progress bars remain on screen after completing.

See the equivalent change in ConcurrentOutput.tsx from #6831 (269f3aa6).

Test plan

  • Tasks.test.tsx — 13 tests pass
  • SingleTask.test.tsx — 11 tests pass
  • Manual: run shopify kitchen-sink async and verify progress bars clear on completion

🤖 Generated with Claude Code

@nickwesselman nickwesselman marked this pull request as ready for review March 20, 2026 16:20
@nickwesselman nickwesselman requested a review from a team as a code owner March 20, 2026 16:20
@github-actions

This comment has been minimized.

The Ink 6 / React 19 upgrade (269f3aa) deferred unmountInk() in
ConcurrentOutput to let React 19 flush batched state updates, but
missed the same pattern in useAsyncAndUnmount (used by Tasks) and
SingleTask. Without the deferral, unmountInk() fires before the
setState that triggers `return null` is flushed, so the final render
still contains the LoadingBar and it is never erased.

Wrap unmountInk() in setImmediate() in both places, matching the
existing fix in ConcurrentOutput.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@nickwesselman nickwesselman force-pushed the fix/progress-bar-clearing-react19 branch from 465d331 to 55dbfc8 Compare March 20, 2026 16:24
@nickwesselman nickwesselman requested a review from a team as a code owner March 20, 2026 16:26
Copy link
Contributor

@ryancbahan ryancbahan left a comment

Choose a reason for hiding this comment

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

Let a note in Slack -- we should merge to fix regression, since this matches code we shipped elsewhere. but this manual management of Node event loop within a React side effect to handle mounting/unmounting feels...very cursed. I'd like us to come back to this.

Copy link
Contributor Author

nickwesselman commented Mar 20, 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.

2 participants