[GH-ISSUE #288] Framework-specific lifecycle dispose missing on destroy #73

Closed
opened 2026-03-02 23:44:21 +03:00 by kerem · 10 comments
Owner

Originally created by @veracioux on GitHub (Nov 9, 2025).
Original GitHub issue: https://github.com/anomalyco/opentui/issues/288

While writing TUI tests for opencode, I noticed that the onCleanup (solidjs) was only invoked when the process exited. This caused a lot of test flakiness because the components had event listeners that survived between tests.

I think that this kind of cleanup should be performed when the renderer is destroyed. I'll create a PoC PR.

Originally created by @veracioux on GitHub (Nov 9, 2025). Original GitHub issue: https://github.com/anomalyco/opentui/issues/288 While writing TUI tests for opencode, I noticed that the onCleanup (solidjs) was only invoked when the process exited. This caused a lot of test flakiness because the components had event listeners that survived between tests. I think that this kind of cleanup should be performed when the renderer is destroyed. I'll create a PoC PR.
kerem closed this issue 2026-03-02 23:44:21 +03:00
Author
Owner

@kommander commented on GitHub (Nov 10, 2025):

@msmps @Adictya can you weigh in here for the framework packages?

<!-- gh-comment-id:3508971928 --> @kommander commented on GitHub (Nov 10, 2025): @msmps @Adictya can you weigh in here for the framework packages?
Author
Owner

@Adictya commented on GitHub (Nov 10, 2025):

the api makes sense for solid, solid-js/web also returns a dispose method like its suggested in the poc

<!-- gh-comment-id:3509567045 --> @Adictya commented on GitHub (Nov 10, 2025): the api makes sense for solid, solid-js/web also returns a dispose method like its suggested in the poc
Author
Owner

@veracioux commented on GitHub (Nov 11, 2025):

@kommander Do I have the green light to implement it, at least for solidjs? I would like to have it available for the opencode tests.

<!-- gh-comment-id:3516521470 --> @veracioux commented on GitHub (Nov 11, 2025): @kommander Do I have the green light to implement it, at least for solidjs? I would like to have it available for the opencode tests.
Author
Owner

@kommander commented on GitHub (Nov 11, 2025):

Sure, seems like it makes sense. Shouldn't break current behavior and tests, right?

<!-- gh-comment-id:3516624621 --> @kommander commented on GitHub (Nov 11, 2025): Sure, seems like it makes sense. Shouldn't break current behavior and tests, right?
Author
Owner

@veracioux commented on GitHub (Nov 11, 2025):

@kommander No it shouldn't. The opencode tests definitely work with the poc from the PR cuz I've been using the development version of opentui from the poc PR branch for a while.

<!-- gh-comment-id:3516638631 --> @veracioux commented on GitHub (Nov 11, 2025): @kommander No it shouldn't. The opencode tests definitely work with the poc from the PR cuz I've been using the development version of opentui from the poc PR branch for a while.
Author
Owner

@veracioux commented on GitHub (Nov 11, 2025):

@kommander Could you also give feedback on this in the PR: https://github.com/sst/opentui/pull/289

A potential alternative would be to add a dispose or onDestroy parameter to CliRendererConfig.

<!-- gh-comment-id:3516658216 --> @veracioux commented on GitHub (Nov 11, 2025): @kommander Could you also give feedback on this in the PR: https://github.com/sst/opentui/pull/289 > A potential alternative would be to add a dispose or onDestroy parameter to `CliRendererConfig`.
Author
Owner

@kommander commented on GitHub (Nov 11, 2025):

Mhh an onDestroy callback like on the renderables could make sense. It needs to be try/catch wrapped though so when something goes wrong in the callback it does not prevent the shutdown from completing, because that would leave the renderer and the terminal in a bad state.

<!-- gh-comment-id:3517519243 --> @kommander commented on GitHub (Nov 11, 2025): Mhh an onDestroy callback like on the renderables could make sense. It needs to be try/catch wrapped though so when something goes wrong in the callback it does not prevent the shutdown from completing, because that would leave the renderer and the terminal in a bad state.
Author
Owner

@veracioux commented on GitHub (Nov 12, 2025):

@kommander Fix for solidjs is ready in https://github.com/sst/opentui/pull/289 (see updated PR description). I'll be playing around with React as well but I'm gonna create a separate PR for that later.

<!-- gh-comment-id:3521028066 --> @veracioux commented on GitHub (Nov 12, 2025): @kommander Fix for solidjs is ready in https://github.com/sst/opentui/pull/289 (see updated PR description). I'll be playing around with React as well but I'm gonna create a separate PR for that later.
Author
Owner

@msmps commented on GitHub (Nov 12, 2025):

@kommander Fix for solidjs is ready in #289 (see updated PR description). I'll be playing around with React as well but I'm gonna create a separate PR for that later.

i've made some changes to the react package and createRoot now returns an unmount method so this should be handled at the consumer level.

i've also introduced the testRenderer for consumers here.

<!-- gh-comment-id:3524134776 --> @msmps commented on GitHub (Nov 12, 2025): > [@kommander](https://github.com/kommander) Fix for solidjs is ready in [#289](https://github.com/sst/opentui/pull/289) (see updated PR description). I'll be playing around with React as well but I'm gonna create a separate PR for that later. i've made some changes to the react package and `createRoot` now returns an `unmount` method so this should be handled at the consumer level. i've also introduced the `testRenderer` for consumers [here](https://github.com/sst/opentui/blob/613689c1e851719a70a17d771275f369a0d7200d/packages/react/src/test-utils.ts#L10).
Author
Owner

@veracioux commented on GitHub (Nov 13, 2025):

Nice. Closing this then.

<!-- gh-comment-id:3526471071 --> @veracioux commented on GitHub (Nov 13, 2025): Nice. Closing this then.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/opentui#73
No description provided.