sveltejs / svelte

web development for the rest of us
https://svelte.dev
MIT License
79.91k stars 4.25k forks source link

Event is not dispatched after component is loaded #4470

Closed MrSrsen closed 6 months ago

MrSrsen commented 4 years ago

Describe the bug Event from component is not dispatched (from reactive function) after component is loaded.

To Reproduce

https://svelte.dev/repl/2b0b7837e3ba44b5aba8d7e774094bb4?version=3.19.1

Expected behavior

Event is dispatched after component is loaded.

Information about your Svelte project:

Severity

Low, I think.

Additional context

When I call dispatch in the setTimeout() with zero delay then it's working corectlly.

If this is desired behaviour then I think it's appropriate to write notice to documentation:

Conduitry commented 4 years ago

What I think is happening is that instantiating the component is immediately running the $: reactive code, which dispatches the event synchronously, before the parent component attaches the listener. I'm currently leaning towards this being something to document rather than being something to change.

MrSrsen commented 4 years ago

Would it be possible to display console warning if dispatch was called before onMount or before parent attached listener to event?

plibither8 commented 4 years ago

Hi, I've been struggling with issue for a while now and was wondering whether I'm doing anything wrong on my part. Is there a work-around for this, since the behaviour might not be changed.

I've reproduced, in a very simple way, what I would like it to do: https://svelte.dev/repl/2b0b7837e3ba44b5aba8d7e774094bb4?version=3.19.1

MrSrsen commented 4 years ago

I was thinking more about this issue and after reading and searching trough some use-cases in our app I think that current behaviours in more appropriate then behaviour that I was originally excepting.

Now I think it's valid behaviour that reactive statements are not running until onMount is fired. Thanks to that I have chance and time to properly initialise all the properties without reactive calls and I do not have to ignore these "initialising" events before proper initialisation.

To sum it up... Note in the docs will be fully appropriate.

My case would be resolved by adding event call manually to the onMount function.

TylerRick commented 3 years ago

@Conduitry wrote:

What I think is happening is that instantiating the component is immediately running the $: reactive code, which dispatches the event synchronously, before the parent component attaches the listener.

I suspect that is correct. (Confirmed. See logs below.)

Should we update the title (since "event is not dispatched" is not quite accurate)?: Events dispatched from child component get lost because dispatched before event listeners attached in parent component

I would love to see a diagram in the docs that illustrates this sequence of events in detail (issue forthcoming!)...

I'm currently leaning towards this being something to document rather than being something to change.

I'm still not quite convinced. Wouldn't Svelte be better if users didn't have to worry about events getting lost (due to a lifecycle event / component tree initialization step order that they have absolutely no control over), and didn't have to resort to kludges like await tick() (from #5405) in order to dispatch an event from a child (often, an initial value) in a way that the parent actually receives the event?

Or at least provide an elegant/easy option to synchronize/postpone the timing of these things for cases where that is preferred. Maybe something like:

<Child on:fill|deferUntilSelfMounts={onFill} />

, which would instruct Svelte to queue up any fill events dispatched by <Child> until self (parent) has finished mounting (and therefore finished attaching all listeners)?

Or something like:

<Child on:fill={onFill} svelte:deferEventsUntilSelfMounts />

or

<Child on:fill={onFill} on:*|queueUntilMounted />

to queue and defer all events dispatched by <Child>. Pretty ugly, but just brainstorming...

Seems like that should just be the default behavior...

Note: I believe there may be related confusing race condition (not technically a race condition since it's deterministic, but what do you call it?) issues related to the use of use: and bind: in a parent component (issues for these may be forthcoming...)...


@MrSrsen wrote:

I was thinking more about this issue and after reading and searching trough some use-cases in our app I think that current behaviours in more appropriate then behaviour that I was originally expecting.

Now I think it's valid behaviour that reactive statements are not running until onMount is fired.

Technically, that's incorrect (see @Conduitry's explanation for the actual reason for the behavior): reactive statements are run before onMount is fired, as this console output (from my fork of your REPL) shows:

Inner: checking isFilled(Some text): true  [called from $: reactivate statement]

Attached listener: ƒ onFill(e) {
            $$invalidate(0, isFilled = e.detail);
        }

Inner: onMount: NodeList [input]

This also seems to confirm @Conduitry's hypothesis that listener isn't attached in parent until after $: isFilled is called in the child.

Thanks to that I have chance and time to properly initialise all the properties without reactive calls and I do not have to ignore these "initialising" events before proper initialisation.

So it sounds like you would be someone who would actually prefer that the current behavior not change?

I'd be curious to see a concrete example where the current behavior would actually be useful/desirable (and if so, whether the perceived problem might actually be a non-issue in those cases, or easily worked around).

Your original REPL is actually a counterexample/the opposite of what you described here — a case where everything has been sufficiently initialized (the correct values for value and length ("Some text" and 5, respectively) have already been assigned via props by this point) and where it would be more useful to have it always dispatch the fill event, even from the initial invocation of the child's $: statement.

My case would be resolved by adding event call manually to the onMount function.

You mean (in your initial example) calling isFilled from both $: isFilled(value) and from an onMount callback?

Sure, that works, but seems unfortunate that one needs to have that duplication.

Another option (which I like even better) is to use the tick() workaround from #5405: see my updated REPL. Then you don't have to add an onMount callback.


@plibither8

Hi, I've been struggling with issue for a while now and was wondering whether I'm doing anything wrong on my part. Is there a work-around for this, since the behaviour might not be changed.

I'm still curious if there's a better workaround, but my personal favorite so far is the await tick() trick from #5405...

I've reproduced, in a very simple way, what I would like it to do: https://svelte.dev/repl/2b0b7837e3ba44b5aba8d7e774094bb4?version=3.19.1

Oops, looks like you linked to the same REPL as the OP. Did you start from their REPL, make some changes, but forget to save your changes (which would have created a new REPL URL)? Would be interested to see what you were trying to show, about what you would like it to do.

MrSrsen commented 3 years ago

@TylerRick wrote: Your original REPL is actually a counterexample/the opposite of what you described here

Yeah sorry. After some time I realised that my first Svelte codebase was not the best and I think I might wrote a lot of garbage code and a lot of weird workarounds. I just want easy and intuitive solution that will be consistent.

I really like yours suggestion for optional synchronising:

@TylerRick wrote: Or at least provide an elegant/easy option to synchronize/postpone the timing of these things for cases where that is preferred. Maybe something like:

<Child on:fill|deferUntilSelfMounts={onFill} />

because it's expressive in what it will do.

In any case, I don't really well understand Svelte internals so I think I just leave solving of this issue to more experienced. :)

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

georgecrawford commented 2 years ago

I had a similar question, arising from an observation that using the Component API, it’s not possible to register an component.$on(…) callback which will be detect an event that’s dispatched during the synchronous component initialisation.

const component = new Component({
    target: renderTarget,
    props: {
        a_prop: 'a prop value'
    }
});

component.$on('my-event', event => {
    alert(event.detail);
});

Using code like this, if Component dispatches an event in the initial tick that it's constructed in, the event listener callback doesn't get fired.

I've put this in a REPL: https://svelte.dev/repl/b6704c77ec4047029658ecba1d97d027?version=3.48.0. You can see that the Component fires the same event twice, once in the initial constructor, and once after 1 second. Only the second one is seen by the callback.

The comments above suggest this is expected behaviour (both using the API and the declarative syntax). If so, I think a console error if dispatch is called during initialisation would be great, to make this easier to spot and debug.

Ideally, however, the API would support adding event callbacks in the constructor. Could this ever work as a feature?

yuliankarapetkov commented 2 years ago

Any progress on this one? :\

frederikhors commented 6 months ago

Is this fixed in Svelte 5?

PatrickG commented 6 months ago

Is this fixed in Svelte 5?

Since createEventDispatcher() is deprecated in favor of callback-function props, I would say, yes.

frederikhors commented 6 months ago

Is this fixed in Svelte 5?

Since createEventDispatcher() is deprecated in favor of callback-function props, I would say, yes.

Yeah. I can confirm it works with 5.

ptrxyz commented 6 months ago

So will this bug still be present if I use Svelte 5 without runes mode?

frederikhors commented 6 months ago

Nope. For me it works even without runes mode. Only svelte 5.

ptrxyz commented 6 months ago

Nope. For me it works even without runes mode. Only svelte 5. Thanks for trying! I guess'll simply wait for the Svelte 5 release and workaround meanwhile. :)