Notifying trigger in component breaks the trigger about leptos HOT 5 CLOSED

Note that using create_render_effect instead of create_effect here also works, because it runs the effect immediately, rather than waiting a tick. I think the issue is that trigger.notify() tries to run all existing effects, which accidentally removes the queued-up create_effect from the queue. So in that sense, this is a bug.

I can't think of a situation in which you'd actually need to do this, though: effects always run once initially (either immediately for create_render_effect or at the end of the microtask for create_effect) so why would you need to notify on the trigger as well? In general I'd say that mutating a signal or notifying a trigger during rendering is a code smell for this reason.

from leptos.

While I agree that the minimal example is contrived and useless. I encountered the bug in a situation in which I needed a trigger that triggered on certain child components being added or removed (with the trigger itself being provided via context). Hence the notify call in the render code to signal that a child component was mounted.

from leptos.

create_effect(move |_| trigger.notify()); in the component body does also work correctly, and is conceptually the "do this once the component has mounted" call.

So, a list of combinations that work correctly that I've tried:

1. no trigger.notify() in the component body
2. create_render_effect and trigger.notify() in the component body
3. create_effect and create_effect(move |_| trigger.notify())
4. using a signal with value () rather than a trigger

4 suggests to me that this is some small, stupid discrepancy in the implementation of Trigger as opposed to signals, which I've tried to track down for a few hours without much less. I'm going to take another crack at it right now and see if I can nail it down.

from leptos.

... well, sometimes once you figure out what the heck is going on, sometimes it's easier than anticipated.

#2059 should fix this (and also another issue with triggers #1948). Feel free to test your actual use case against that branch and let me know if it fixes it, but I think it will.

from leptos.

Yep, that commit indeed fixes the problem. Thanks a lot for investigating and fixing it!

from leptos.