search

jsonnull / electron-trpc

Build type-safe Electron inter-process communication using tRPC
https://electron-trpc.dev/
MIT License
267 stars 26 forks source link

Reasoning behind `Clean up subscriptions on navigation` #200

Open dastein1 opened 4 months ago

dastein1 commented 4 months ago

I'm using mat-sz:trpc-11's fork in conjunction with trpc@11 in a demo project. PR #165 closes all subscriptions on navigation. For me that closes too many subscriptions on navigation and has the problem that the useSubscription() mechanism doesn't work anymore on those closed subscriptions. What's the reasoning behind that?

jsonnull commented 4 months ago

Subscriptions should be closed when the client closes the subscription, or when the client disconnects.

The problem is that the connection mechanism for electron isn't like browser connections, where when the window hard-navigates a websocket connection is closed, allowing the server to run cleanup.

We attempt to detect when a window hard-navigates so that the server can run subscription cleanup when hard navigation happens.

dastein1 commented 4 months ago

Thanks. I do understand that, but I think that the assumption that a component has to unmount on route change is not true. For those components that do not remount it may completely break the subscriptions as the server then runs some cleanup and useSubscription() is not going to re-establish that connection. Could we make that configurable?

jsonnull commented 4 months ago

I think that the assumption that a component has to unmount on route change is not true

We don't enforce component mounting/unmounting on route change, that's all tRPC.

What we do is listen for window navigation events and manually close subscriptions when we know the page will be dropped. Note that components don't unmount on navigation, as the in-page JavaScript context is no longer executing.

I'm looking at the docs, though, and we're using the wrong event-listener for our navigation cleanup, causing this to fire on in-page navigations when it should only be firing for hard navigation. So that's a bug we'll have to address.