fix(runtime-core): avoid caching unmounted suspense children

Project: vuejs/core | Language: TypeScript | Concepts: caching, error-handling, testing-strategy, distributed-systems
Difficulty: mid
Link: https://github.com/vuejs/core/pull/15291

The problem

When a nested Suspense component is updated while an ancestor Suspense is still pending, the update is intentionally skipped to avoid double-processing. In this case, the new vnode for the nested Suspense child does not have a component instance attached. However, the KeepAlive component still cached this vnode. Later, when trying to reuse this cached vnode, the system would try to access properties on a null component, causing runtime errors (e.g., reading component.suspenseId on null).

Why it's non-trivial

A naive fix might simply try to always cache or not cache vnodes, but this would either not solve the bug (still caching invalid vnodes) or break valid caching scenarios. The fix must precisely detect when a vnode is safe to cache (i.e., it has a valid component instance) and only cache in those cases, without disrupting the intended behavior of Suspense and KeepAlive interactions.

The approach

1. Guard caching in KeepAlive to only cache vnodes with a component instance:

Before:

- cache.set(pendingCacheKey!, getInnerChild(instance.subTree))

This line would cache the inner child of a Suspense subtree, even if it did not have a valid component instance.

After:

+ const vnode = getInnerChild(instance.subTree)
+ if (vnode.component) {
+   cache.set(pendingCacheKey!, vnode)
+ }

Annotation:
Now, before adding the vnode to the cache, the code checks that vnode.component is truthy. This ensures only fully mounted and valid vnodes are cached, preventing future errors when the cache is accessed.

2. Regression test for the bug:

A new test was added to cover the scenario where a nested Suspense is switched while an ancestor is still pending:

+  test('KeepAlive + Suspense switch while ancestor is pending', async () => {
+    // ...setup code...
+    render(h(App), root)
+    expect(serializeInner(root)).toBe('<div>loading</div>')
+
+    await Promise.all(deps)
+    await nextTick()
+
+    page.value = PageB
+    key.value = 'b'
+    await nextTick()
+    expect(serializeInner(root)).toBe('<div>loading</div>')
+
+    resolveRootDep({
+      render: () => h('div', 'root'),
+    })
+    await rootDep
+    await new Promise(r => setTimeout(r))
+    await nextTick()
+    expect(serializeInner(root)).toBe(
+      '<div><div>root</div><div>page A</div></div>',
+    )
+    // ...more switches and checks...
+  })

Annotation:
This test simulates switching between pages inside nested Suspense and KeepAlive components, while the ancestor Suspense is still pending. It checks that the fallback is shown at the right times and that the correct content is rendered after everything resolves, ensuring the bug is fixed.

From the review thread

No notable review discussion on this PR.

Takeaways

• Only cache objects in a component cache if they are fully initialized and valid; otherwise, you risk runtime errors when the cache is accessed.
• When fixing a bug involving async or deferred rendering, always add a regression test that covers the exact sequence of events that triggered the bug.
• Guarding side effects (like caching) with precise checks (e.g., if (vnode.component)) helps maintain correctness in complex component hierarchies.