Framework-specific lifecycle dispose missing on destroy

[veracioux]

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.

[kommander]

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

[Adictya]

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

[veracioux]

@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.

[kommander]

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

[veracioux]

@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.

[veracioux]

@kommander Could you also give feedback on this in the PR: #289

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

[kommander]

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.

[veracioux]

@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.

[msmps]

@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.

[veracioux]

Nice. Closing this then.