fix(scheduler): should insert jobs in ascending order of job's id when flushing

[HcySunYang]

Fix: #2768, Fix #2829

what's happened?

Let's talk about what happened first, well, they are very complex issues(#2768, #2829). I tried to point out the minimum reproduction, check out https://codesandbox.io/s/nifty-glitter-u11y9?file=/src/index.ts, it's based on #2768. I will use the reproduced code to illustrate what happened.

Take a look at the complete code:

const Wrap = defineComponent({ render(){ return this.$slots.default!() } })

const Test = defineComponent({
  setup() {
    const outerC = inject('outerC')! as any

    return () => h('div', `injected: ${outerC.value}`)
  }
})

const IssueComp = defineComponent({
  props: ['issueShow'],
  setup(props) {
    return () => h("div", "I should disappear (show: " + props.issueShow + ")")
  }
})

const Inner = defineComponent({
  name: 'Inner',
  props: ['innerShow'],
  setup(props) {
    return () => (openBlock(), createBlock("div", null, [
      createVNode(Wrap, null, {
        default: () => [
          createVNode(IssueComp, { issueShow: props.innerShow }, null, 8 /* PROPS */, ["issueShow"])
        ],
        _: 1 /* STABLE */
      })
    ]))
  }
})

const Outer = defineComponent({
  name: 'Outer',
  props: ['outerShow'],
  setup(props) {
    const outerCompute = computed(() => props.outerShow)

    provide('outerC', outerCompute)

    return () => (openBlock(), createBlock(Fragment, null, [
      createVNode(Inner, { innerShow: props.outerShow }, null, 8 /* PROPS */, ["innerShow"]),
      createVNode(Wrap, null, {
        default: withCtx(() => [
          createVNode(Test, { testShow: props.outerShow }, null, 8 /* PROPS */, ["testShow"])
        ]),
        _: 1 /* STABLE */
      }),
      createTextVNode(" " + toDisplayString(outerCompute.value), 1 /* TEXT */)
    ], 64 /* STABLE_FRAGMENT */))
  }
})

const Root = defineComponent({
  setup() {
    const rootShow = ref(true)

    setTimeout(() => {
      rootShow.value = false
    }, 1000);

    return () => (openBlock(), createBlock(Outer, { outerShow: rootShow.value }, null, 8 /* PROPS */, ["outerShow"]))
  }
})

createApp(Root).mount('#header')

Yes, it contains some code generated by the compiler (such as createBlock etc.), this is because we have to reproduce the problem based on that.

There are 6 components here. After the first rendering is completed, 7 component instances will be created(the Wrap component rendered twice), there is also a computed value(outerCompute), they all create an effect function, and the ids of these effect functions are incremented in the order they were created. We can use Comp(effect id) to express, for example, Root(0) represents the render effect function of the Root component, and its id is 0, the complete list is:

• Root(0)
• Computed(1) - computed's effect
• Outer(2)
• Inner(3)
• Wrap(4) - as Inner's child
• IssueComp(5) - It is the problematic component
• Wrap(6) - as Outer's child
• Test(7)

Start by toggle the value of rootRef:

rootRef.valjue = false

Will do these detailed steps:

1. set `rootRef.value = false`
  - Only the `Root(0)` is tracked by `rootRef`, so `queueJob(Root(0))`
  - The current queue is `[Root(0)]`
  - Start flushing the queue, and the `flushIndex` is `0`
  - Run job `Root(0)`, patching `Root`
    2. Should update `Outer` ?
      - yes, then, set `props.outerShow = false`
      - Both `Computed(1)` and `Wrap(6)` are tracked by the `outerShow`
      - So, run `Computed(1)`, and it will trigger `queueJob(Test(7))`
      - The current queue is `[Root(0), Test(7)]`
      - then, `queueJob(Wrap(6))`
      - The current queue is `[Root(0), Test(7), Wrap(6)]`
      - then, patching `Outer`
        3. Should update `Inner` ?
          - yes, then, set `props.innerShow = false`
          - Only `Wrap(4)` is tracked by `innerShow`
          - So, `queueJob(Wrap(4))`
          - The current queue is `[Root(0), Test(7), Wrap(6), Wrap(4)]`
          - then, patching `Inner`
            4. Should update `Wrap(4)` ?
              - no
          5. Should update `Wrap(6)` ? 
            - no
  - flushIndex --> 1
    6. The current queue is `[Root(0), Test(7), Wrap(6), Wrap(4)]`
      - The job with index `1` is `Test(7)`
      - So, run `Test(7)`
      - patcing `Test`
  - flushIndex --> 2
    7. The current queue is `[Root(0), Test(7), Wrap(6), Wrap(4)]`
      - The job with index `2` is `Wrap(6)`
      - So, run `Wrap(6)`
      - patcing `Wrap`
        8. Should update `Test(7)` ?
          - yes
          - `invalidateJob(Test(7))`
          - The current queue is: `[Root(0), Wrap(6), Wrap(4)]`, `Test(7)` be removed.
          - patcing `Test`
  - flushIndex --> 3
    9. The current queue is: `[Root(0), Wrap(6), Wrap(4)]`
      - Which job has an index of `3`?
      - So, end

** `Wrap(4)` was skipped, which means the `IssueComp` will not be updated **

Conclusion: Wrap(4) was skipped, which means the IssueComp will not be updated

Step 8 is the problematic point:

        8. Should update `Test(7)` ?
          - yes
          - `invalidateJob(Test(7))`
          - The current queue is: `[Root(0), Wrap(6), Wrap(4)]`, `Test(7)` be removed.
          - patcing `Test`

invalidateJob(Test(7)) shortens the queue's length, but flushIndex remains the same, which makes subsequent tasks skipped(Wrap(4)).

Of course we can use the changes introduced by @edison1105 in its PR(#2834) to fix the bug, but please pay attention to the above steps, there are still problems there: Test(7) was patched twice because it entered the queue prematurely:

Do the first patch in step 6:

    6. The current queue is `[Root(0), Test(7), Wrap(6), Wrap(4)]`
      - The job with index `1` is `Test(7)`
      - So, run `Test(7)`
      - patcing `Test`  // here

Made the second patch in step 8:

        8. Should update `Test(7)` ?
          - yes
          - `invalidateJob(Test(7))`
          - The current queue is: `[Root(0), Wrap(6), Wrap(4)]`, `Test(7)` be removed.
          - patcing `Test`  // here

You can verify it by modifying the Test component:

const Test = defineComponent({
  setup() {
    const outerC = inject('outerC')! as any
    // the `run` fn will run twice
    const run = () => { console.log('test run') }

    return () => h('div', `injected: ${outerC.value}, ${run()}`)
  }
})

How to fix that?

You may have noticed that the order in which jobs are enqueued is not the order of increasing id: [Root(0), Test(7), Wrap(6), Wrap(4)], but when we start to flush the queue, we expect jobs to be executed in ascending order of id, so we can keep the order when the job is enqueued, which can prevent the job from being skipped and also can avoid repeated patching. We can use binary-search to find a suitable position in the queue. I think its cost is less than the cost of repeated patching.

[HcySunYang]

CI failed due to E2E testing

[LinusBorg]

For reference: This is the previous PR by @edison1105 that attempts to solve this problem as well:

#2834

@yyx990803 your input here would be welcome as it's a very complex issue touching the scheduler internals.