tauri icon indicating copy to clipboard operation
tauri copied to clipboard

fix(event): clear residual js listeners (#8916 )

Open canxin121 opened this issue 1 year ago • 1 comments

fix clear residual listeners, close #8916

I noticed that in v1, js_listeners were stored on the frontend, so refreshing the page cleans up the original js_listeners (note that this doesn't mean that useEffect cleans up the side-effects, as it doesn't have a way to clean up the side-effects before refreshing the page).

However, in v2, js_listeners are stored not only in the front end, but also in the back end, and the number of events triggered is determined by the js_listeners that stored in the backend, which results in a pileup of js_listeners that can't be cleaned up when the page is refreshed.

I added a new js command unlisten_all and invoke it at the end of core.js to solve this problem. unlisten_all only cancels the js_listeners at the frontend and backend of the current window.

This fix works fine on my machine, in both single-window and multi-window scenarios.

canxin121 avatar Feb 21 '24 11:02 canxin121

A fixed demo

https://github.com/tauri-apps/tauri/assets/69547456/b1a34d18-f9f7-43fe-b918-867ca651a0ae

The origin error demo can be seen at #8916

canxin121 avatar Feb 21 '24 12:02 canxin121

I think there's a better way to do this. Handling this via a command called by the frontend might lead to race conditions. What we could do instead is modify this closure and clear the listeners when the page starts to load.

lucasfernog avatar Feb 26 '24 16:02 lucasfernog

I think there's a better way to do this. Handling this via a command called by the frontend might lead to race conditions. What we could do instead is modify this closure and clear the listeners when the page starts to load.

Very good suggestion, I hadn't thought of fixing this bug here before, and now that I've rewritten a new version which doesn't involve any front-end code and no more unlisten_all_js for tauri::command, it should work as expected.

https://github.com/tauri-apps/tauri/assets/69547456/b003c082-578d-4122-97f9-4ea591140c9d

canxin121 avatar Feb 27 '24 10:02 canxin121

Been using this for today's development and had no issues at all :)

vonPB avatar Feb 27 '24 23:02 vonPB

Thank you

amrbashir avatar Feb 28 '24 18:02 amrbashir

I'm not sure if this is expected, but this change seems to be causing some kind of race condition on initial load of a React page. I am calling listen() from the componentDidMount() function of a React component. From my debugging it seems like the following order of events is happening:

  1. Page begins to load, componentDidMount() is called, my code calls listen() and registers a listener
  2. Tauri considers the page to be "loaded". The on_page_load_handler is called, and hits a breakpoint inside the function that was added in this PR, unlisten_all_js(). This clears the listener my page had registered. (I verified this by inspecting the members in the debugger)
  3. Later, my backend code emits some events, but there is nothing to listen to them.

I am new to Tauri, so I may be doing something wrong, but is this expected?

samkearney avatar Mar 08 '24 19:03 samkearney

Hmm, i highly doubt that componentDidMount triggers before DOMContentLoaded. Not 100% sure but still feels wrong to me.

Can you share the code where you listen to and emit the event?

FabianLars avatar Mar 08 '24 19:03 FabianLars

It's closed-source, but let me try to reduce it to something minimal. While I work on that, could you point me to any advice or best practices for places in the lifecycle of a React page where it makes the most sense to set up event listening?

samkearney avatar Mar 08 '24 19:03 samkearney

componentDidMount or useEffect with [] is indeed correct. You just have to unlisten to the event on unmount too. An example for that with useEffect (because i didn't use componentDidMount for years) looks like this

useEffect(() => {
  const unlisten = listen("someevent", console.log);
  
  return () => {
    unlisten.then( f => f() );
  }
}, []);

FabianLars avatar Mar 08 '24 20:03 FabianLars

In making my minimal reproducer I realized I had several incorrect assumptions about my code. I think things are working now. Apologies for the spam.

samkearney avatar Mar 08 '24 21:03 samkearney

Here's a repro demonstrating a bug where the listeners are cleaned up too late, destroying new callbacks that were just installed. For me, this works on Windows and on MacOS dev builds, but fails on MacOS production builds. https://github.com/gulbanana/repro-tauri-macos-events

gulbanana avatar Mar 10 '24 09:03 gulbanana