Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

calling listen() emits events from the past #2323

Closed
FabianLars opened this issue Jul 29, 2021 · 1 comment
Closed

calling listen() emits events from the past #2323

FabianLars opened this issue Jul 29, 2021 · 1 comment

Comments

@FabianLars
Copy link
Member

This will be slightly longer as i'm not able to formulate concise sentences...

Describe the bug

Finally got the time to convert my discord message (https://discord.com/channels/616186924390023171/731495028677148753/869314406075498516) into an issue.

First of all the steps to reproduce (with tauri://move as an example event):

  1. Have some kind of tauri app, without a event listener for the event that we want to test here. You might want to enable withGlobalTauri for this.
  2. Move the window around like crazy.
  3. Open the dev console and write window.__TAURI__.event.listen('tauri://move', console.log)
  4. Enjoy the console spam (we're talking about potentially 1000+ events)
    => I didn't test this for event listeners on the rust side (yet).

I found out about that while i was integrating a file-drop handler in react, where a component was able to receive files that were dropped on the window before the component was mounted (and therefore before it started listening to events).

Searching for the root cause of this i found this: event.rs and maybe this: manager.rs (and the loop below that).
I wanted to create a PR for that, but didn't know what the best fix would be. I don't really understand why we would want to loop through the stored events on listen() anyway as it doesn't sound like expected behaviour on listener registration for me.
Took me some time to realize, but what's even weirder for me is that we actually have events for that loop to iterate on.

Speaking of stored events, these also never get removed from the array, therefore the array will get realllyyyyyy big really quickly (some resizing and some moving and we're easily at 10k stored events, as these event types are quite spammy).
(The array is accessible via window[some_random_id] )

So, what would be the best way to fix this?
Removing the for loop as I don't see a use case for emitting events from the past on listener registration?
Only store events in that array if there is an listener for that specific event registered?
Both?
On top of that i think it would be nice to remove events from the array after "use", but i'm not sure if this would be trivial as you could have multiple listeners registered for a single event.

Expected behavior

The event handler should only be called for events after the listen() call.

Platform and Versions (required):

current dev branch and:

Operating System - Windows, version 10.0.19043 X64
Webview2 - 92.0.902.55

Node.js environment
  Node.js - 16.4.2
  @tauri-apps/cli - 1.0.0-beta.6
  @tauri-apps/api - 1.0.0-beta.5

Global packages
  npm - 7.18.1
  yarn - 1.22.10

Rust environment
  rustc - 1.53.0
  cargo - 1.53.0

App
  tauri.rs - 1.0.0-beta.5
@lucasfernog
Copy link
Member

We have this event queue that are no longer needed. Removing it will fix this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants