fix(runtime-core): invalidating jobs with duplicates

[skirtles-code]

The scheduler queue can contain duplicates of the same job in some circumstances, i.e. if the job gets added again while the queue is flushing, after it has already run the first time.

invalidateJob() currently doesn't take account of that, just searching for the first index:

const i = queue.indexOf(job)

This could be fixed using lastIndexOf(), but instead I've added a start index to the indexOf() call. This should be sufficient, as a job should only appear once in the unprocessed part of the queue.

Adding an index should theoretically make the indexOf() a bit faster, though in practice it's unlikely to matter.

The incorrect invalidation logic can lead to components being rendered unnecessarily. In practice this is quite difficult to hit in real code, as it takes some quite specific circumstances for invalidateJob() to be called with duplicate jobs in the queue:

SFC Playground

In the example above, clicking the button logs Child rendering 3 times, when it should just be logged twice.

The same example with this PR:

SFC Playground - Fixed

[sxzz]

/ecosystem-ci run

[vue-bot]

📝 Ran ecosystem CI: Open

suite	result
nuxt	✅ success
pinia	✅ success
router	✅ success
test-utils	✅ success
vite-plugin-vue	✅ success
vitepress	✅ success
vue-macros	✅ success
vueuse	✅ success

[github-actions]

Size Report

Bundles

File	Size	Gzip	Brotli
runtime-dom.global.prod.js	89.5 kB (+5 B)	34 kB (+6 B)	30.7 kB (+28 B)
vue.global.prod.js	146 kB (+5 B)	53.3 kB (+6 B)	47.6 kB (-25 B)

Usages

Name	Size	Gzip	Brotli
createApp	49.8 kB (+5 B)	19.5 kB (+5 B)	17.8 kB (+8 B)
createSSRApp	53.2 kB (+5 B)	20.8 kB (+5 B)	19 kB (+2 B)
defineCustomElement	52.1 kB (+5 B)	20.3 kB (+5 B)	18.5 kB (+16 B)
overall	63.3 kB (+5 B)	24.5 kB (+5 B)	22.2 kB (-27 B)

[edison1105]

SFC Playground
In the example above, clicking the button logs Child rendering 3 times, when it should just be logged twice.

It only logs twice for now, maybe that already fixed via some commits.

[edison1105]

/ecosystem-ci run

[vue-bot]

📝 Ran ecosystem CI: Open

suite	result	latest scheduled
language-tools	❌ failure	✅ success
nuxt	✅ success	✅ success
pinia	✅ success	✅ success
primevue	✅ success	✅ success
quasar	✅ success	✅ success
radix-vue	✅ success	✅ success
router	✅ success	✅ success
test-utils	✅ success	✅ success
vant	✅ success	✅ success
vite-plugin-vue	✅ success	✅ success
vitepress	✅ success	✅ success
vue-i18n	✅ success	✅ success
vue-macros	✅ success	✅ success
vuetify	✅ success	✅ success
vueuse	❌ failure	✅ success
vue-simple-compiler	✅ success	✅ success

[skirtles-code]

It only logs twice for now, maybe that already fixed via some commits.

Good point.

I did some digging... this is a bit difficult to explain, but I'll try my best.

invalidateJob still contains the original bug, that hasn't been fixed on main. The test case I added still fails on main and passes with my fix.

The reactivity system improvements in 3.4 are hiding the issue in my original example. The wrong job gets invalidated, but it gets away with it because the component doesn't actually end up rendering.

invalidateJob is only used in one place:

core/packages/runtime-core/src/renderer.ts

Line 1291 in cf8cd1d

invalidateJob(instance.update)

So the only way to hit invalidateJob in application code is via an update job. But in 3.4, that update job was changed to this:

core/packages/runtime-core/src/renderer.ts

Lines 1585 to 1589 in cf8cd1d

	const update: SchedulerJob = (instance.update = () => {
	if (effect.dirty) {
	effect.run()
	}
	})

This is why my original example no longer renders 3 times, because the effect.dirty check prevents the third render. The job is still incorrectly queued, but the dirtiness check stops it running.

This raises another question: do we actually need to invalidate the update job in the first place? If the dirty flag stops it running then why bother?

So, in summary...

1. I suspect invalidateJob can just be removed, as we don't actually need it. But...
2. If there is a reason why it is still needed, then my fix is still needed to ensure it invalidates the correct job.

[edison1105]

@skirtles-code
Thanks for your explanation. I marked it as need guidance, waiting for Evan's input.