Tear down with useEffect cleanup functions

[groner]

It seems that using screen.destroy() to tear down my application doesn't cause useEffect cleanup functions to be called.

Right now I'm working around with a wrapper container that I can tell to de-render its children prior to calling screen.destroy. Is there a better way?

I'm using the 0.7.0 release.

[Yomguithereal]

Hello @groner, frankly I am a bit amazed that cleanup functions are run on screen.destroy since I don't even know how react detects this. I guess this is tied to blessed event emitters. But anyway, can you provide a minimal reproducible use case so I can try to tweak this?

On a side note, how would react-dom work with this? My point is, isn't what you observe the "expected" behavior?

[groner]

Hi @Yomguithereal,

Thanks for looking at this, and also for this project.

I guess my wording may have been less than clear, but the issue is that useEffect cleanup functions aren't run when screen.destroy is called.

I've created a reproduction. Its a little longish, so I've put it in a gist. The gist contains a script demonstrating the issue, and a second script demonstrating the workaround I'm currently using.

For my current needs it's certainly possible for my application to do screen.destroy(); process.exit(); and not worry about any open resources. I'm generally interested in accounting for running timers and open connections etc in order to know that I'm not leaking them (this seems to be very easy to get wrong in react). If all the effects shutdown cleanly, then node will just exit when there are no active resources left.

Regarding react-dom, I think the browser environment just doesn't provide a means to defer unloading while asynchronous cleanup functions run. I guess react could run synchronous cleanup function in a beforeunload event handler. I haven't used react in the browser yet, so I don't know if that's the case or not.

I'd like to suggest that react-blessed should provide some mechanism that makes this easy to get right by default. It doesn't matter to me if it works with screen.destroy, but it would be nice if it didn't require opting in with a component like the <Unmounter/> in my work around.

[Yomguithereal]

Hello @groner, I see your point. Let me try some things but I am not sure I can do anything without monkey-patching your screen's destroy function which feels a bit too dangerous to my taste. By maybe I can rely on some event or some dummy root element for this, but the fact is I will probably not be able to ensure some async cleanup is run to completion in this case.

[Yomguithereal]

Hi again @groner, I have tried something on this commit. Can you try it and tell me if this could fit the bill?

[groner]

Hi @Yomguithereal,

This change seems good. My application is able to cleanup and exit without using the Unmounter wrapper.

[Yomguithereal]

Ok, v0.7.1 is live on npm with the fix. Will close this issue then.