sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.52k stars 4.12k forks source link

afterUpdate callbacks fire in different order in onMount vs later updates #7001

Open rmunn opened 2 years ago

rmunn commented 2 years ago

Describe the bug

While working on https://github.com/sveltejs/svelte/pull/6920, I discovered that the order of calling afterUpdate callbacks is inconsistent. During initial component mounting, afterUpdate callbacks from child components will be called before afterUpdate callbacks from parent components. But during updates that happen after mounting (e.g., adding 1 to a count variable), the afterUpdate callbacks from parent components are called first, then afterUpdate callbacks from child components are called later.

This may or may not be intended behavior, but it seems strange to me that the callback order is different at mount time vs. when "regular" updates happen.

Reproduction

https://svelte.dev/repl/4b285f9150ed4812a7205291e10b3b86?version=3.44.2

Expected:

"Registering callbacks in parent"
"Parent beforeUpdate, count is 0"
"Registering callbacks in child #1"
"Child #1 beforeUpdate, count is 0"
"Registering callbacks in child #2"
"Child #2 beforeUpdate, count is 0"
"Child #1 onMount"
"Child #1 afterUpdate, count is 0"
"Child #2 onMount"
"Child #2 afterUpdate, count is 0"
"Parent onMount"
"Parent afterUpdate, count is 0"
(At this point I clicked the "Add 1 to count" button)
"Parent beforeUpdate, count is 1"
"Child #1 beforeUpdate, count is 1"
"Child #2 beforeUpdate, count is 1"
"Child #1 afterUpdate, count is 1"
"Child #2 afterUpdate, count is 1"
"Parent afterUpdate, count is 1"

Actual: almost identical to what I expected to see, except for the last three lines, which were:

"Parent afterUpdate, count is 1"
"Child #1 afterUpdate, count is 1"
"Child #2 afterUpdate, count is 1"

See Logs section below for the full logs from the Svelte REPL console.

Logs

"Registering callbacks in parent"
"Parent beforeUpdate, count is 0"
"Registering callbacks in child #1"
"Child #1 beforeUpdate, count is 0"
"Registering callbacks in child #2"
"Child #2 beforeUpdate, count is 0"
"Child #1 onMount"
"Child #1 afterUpdate, count is 0"
"Child #2 onMount"
"Child #2 afterUpdate, count is 0"
"Parent onMount"
"Parent afterUpdate, count is 0"
(At this point I clicked the "Add 1 to count" button)
"Parent beforeUpdate, count is 1"
"Child #1 beforeUpdate, count is 1"
"Child #2 beforeUpdate, count is 1"
"Parent afterUpdate, count is 1"
"Child #1 afterUpdate, count is 1"
"Child #2 afterUpdate, count is 1"

System Info

REPL. Svelte version 3.44.2.

Severity

annoyance

rmunn commented 2 years ago

Note that when I edit the REPL and trigger the onDestroy callbacks because the DOM is being updated with my new code, the onDestroy callbacks fire in the following order:

"Parent onDestroy"
"Child #1 onDestroy"
"Child #2 onDestroy"

I'm not sure if this should be the same order as onMount or the reverse order. For onMount it makes sense that the parent's callback would fire last, so that by the time the parent onMount runs, all its children have been fully mounted and (for example) their bind:this bindings are available to the parent. But when destroying the components, which of these two options makes the most sense?

  1. The parent's onDestroy should fire first, so that its children are still mounted at the time onDestroy runs. Then the children's onDestroy fires, at which time they have no parent component.
  2. The children's onDestroy fires first, so that their parent component still exists at the time onDestroy runs. Then the parent's onDestroy fires, at which time its child components have already been destroyed.

I think option 1 (the current behavior) makes the most sense now that I've thought about it, since in onDestroy you might want to undo what you did in onMount. So I don't think that call order is a bug. But I wanted to mention this in case others think option 2 makes more sense.

rmunn commented 2 years ago

Related to https://github.com/sveltejs/svelte/pull/3150 and https://github.com/sveltejs/svelte/issues/2281. It's clear from #3150 that the intended order for afterUpdate during mounting is that children should fire afterUpdate before parents do, and that's indeed what I'm seeing. But it seems to me that the same call order would make sense in "normal" updates, and that's not happening.