Duplicate execution in post-flush queue

[jods4]

Version

3.0.0-beta.22

Reproduction link

https://github.com/vuejs/vue-next/blob/master/packages/runtime-core/src/scheduler.ts#L55-L56

Steps to reproduce

Sorry, no reproduction this time.
I was discussing the reactivity queues with @RobbinBaauw when we noticed something strange in the source code.

The scheduler queue (pre) and postFlushCbs (post, default for watches) queues are implemented quite differently and they both seem to have potential bugs.

The worse one is postFlushCbs:
Because it dedupes and then copies the current queue before flushing it, if a watch already in queue is triggered during flush, it will be executed a second time (added to now empty postFlushCbs, when current flush ends a second one will start immediately).

The queue defect seems more unlikely.
Jobs are deduped at enqueue time and the queue is shifted in a loop so there's no duplicate execution possible.
The execution is supposed to be ordered by effect id, though (to guarantee top-down rendering and avoid bugs related to v-if and nulls state in children) but that ordering is defined by a sort at the beginning of the flush loop.
If new jobs are added during flush, they'll execute (as long as they're not already in queue) but always last, regardless of their effect id.

What is expected?

Watches should not execute twice without a change in between.

Renders should always execute top-down (in effect id order).

What is actually happening?

Both of those conditions can be broken when enqueuing during a flush.

[yyx990803]

The queue ordering has some internal assumptions: a new main queue job is only queued during the flush as a parent -> child forced update, for which the child is guaranteed to have a bigger ID than the parent (the sorting is there exactly to ensure child is updated after the parent).

The only edge case for this is when user triggers state mutations in a component's own beforeXXX hooks, which should be avoided anyway.

[basvanmeurs]

@yyx990803 @jods4 @RobbinBaauw

Even though the commit 165068d by @yyx990803 mitigates the problem it doesn't fix it for multi-iteration effect queue drains.

My proposition is to add a new flush mode in the WatcherOptions named 'final', which is executed after all other effects in multi-iteration event flushes.

I've implemented this in this PR: #1599

[RobbinBaauw]

165068d may have introduced another problem (it broke our custom renderer). flushPostFlushCbs is called in the render function: https://github.com/vuejs/vue-next/blob/master/packages/runtime-core/src/renderer.ts#L2125-L2135.

The problem lies in the fact that our custom renderer was called in a watchEffect (https://github.com/Planning-nl/vugel/blob/ace01bdb00a951801bd352da8a88595f7587a5f0/src/wrapper.ts#L39-L68), meaning that the following would happen:

1. root render called
2. `flushPostFlushCbs` called
  2a. our `watchEffect` with render called
  2b. `flushPostFlushCbs` called again
  2c. `flushPostFlushCbs` finishes
3. `flushPostFlushCbs` continues, but `pendingPostFlushCbs` is now `null`!

We fixed this by waiting for nextTick and this may be our problem and not something Vue should deal with, but just wanted to put this out there.