From ebc9d9ff0d1ebfe9ad14d3efceb48fa0251bd5f9 Mon Sep 17 00:00:00 2001 From: edison1105 Date: Tue, 9 Jan 2024 21:51:29 +0800 Subject: [PATCH 1/6] fix(Suspense): avoid patch nested suspense with suspensible true --- .../__tests__/components/Suspense.spec.ts | 143 ++++++++++++++++++ .../runtime-core/src/components/Suspense.ts | 4 + 2 files changed, 147 insertions(+) diff --git a/packages/runtime-core/__tests__/components/Suspense.spec.ts b/packages/runtime-core/__tests__/components/Suspense.spec.ts index 203937aa526..0141a22ca87 100644 --- a/packages/runtime-core/__tests__/components/Suspense.spec.ts +++ b/packages/runtime-core/__tests__/components/Suspense.spec.ts @@ -1523,6 +1523,149 @@ describe('Suspense', () => { expect(calls).toContain('innerB mounted') }) + //#8678 + test('avoid patch nested suspense with suspensible', async () => { + const calls: string[] = [] + let expected = '' + + const InnerA = defineAsyncComponent( + { + setup: () => { + calls.push('innerA created') + onMounted(() => { + calls.push('innerA mounted') + }) + return () => h('div', 'innerA') + }, + }, + 10, + ) + + const InnerB = defineAsyncComponent( + { + setup: () => { + calls.push('innerB created') + onMounted(() => { + calls.push('innerB mounted') + }) + return () => h('div', 'innerB') + }, + }, + 10, + ) + + const OuterA = defineAsyncComponent( + { + setup: (_, { slots }: any) => { + calls.push('outerA created') + onMounted(() => { + calls.push('outerA mounted') + }) + return () => + h(Fragment, null, [h('div', 'outerA'), slots.default?.()]) + }, + }, + 5, + ) + + const OuterB = defineAsyncComponent( + { + setup: (_, { slots }: any) => { + calls.push('outerB created') + onMounted(() => { + calls.push('outerB mounted') + }) + return () => + h(Fragment, null, [h('div', 'outerB'), slots.default?.()]) + }, + }, + 5, + ) + + const outerToggle = ref(false) + const innerToggle = ref(false) + + /** + * + * + * + * + * + * + * + */ + const Comp = { + setup() { + return () => + h(Suspense, null, { + default: [ + h(outerToggle.value ? OuterB : OuterA, null, { + default: () => + h( + Suspense, + { suspensible: true }, + { + default: h(innerToggle.value ? InnerB : InnerA), + }, + ), + }), + ], + fallback: h('div', 'fallback outer'), + }) + }, + } + + expected = `
fallback outer
` + const root = nodeOps.createElement('div') + render(h(Comp), root) + expect(serializeInner(root)).toBe(expected) + + // mount outer component + await Promise.all(deps) + await nextTick() + + expect(serializeInner(root)).toBe(expected) + expect(calls).toEqual([`outerA created`]) + + // mount inner component + await Promise.all(deps) + await nextTick() + expected = `
outerA
innerA
` + expect(serializeInner(root)).toBe(expected) + + expect(calls).toEqual([ + 'outerA created', + 'innerA created', + 'outerA mounted', + 'innerA mounted', + ]) + + calls.length = 0 + deps.length = 0 + + // toggle both outer and inner component + outerToggle.value = true + innerToggle.value = true + await nextTick() + + await Promise.all(deps) + await nextTick() + expect(serializeInner(root)).toBe(expected) // expect not change + + await Promise.all(deps) + await nextTick() + expected = `
outerB
innerB
` + expect(serializeInner(root)).toBe(expected) + + // innerB only mount once + expect(calls).toEqual([ + 'outerB created', + 'innerB created', + 'outerB mounted', + 'innerB mounted', + ]) + }) + // #8206 test('nested suspense with suspensible & no async deps', async () => { const calls: string[] = [] diff --git a/packages/runtime-core/src/components/Suspense.ts b/packages/runtime-core/src/components/Suspense.ts index 8fbb07de9a1..c3b66e4d813 100644 --- a/packages/runtime-core/src/components/Suspense.ts +++ b/packages/runtime-core/src/components/Suspense.ts @@ -91,6 +91,10 @@ export const SuspenseImpl = { rendererInternals, ) } else { + if (isVNodeSuspensible(n2) && parentSuspense?.deps !== 0) { + n2.suspense = n1.suspense + return + } patchSuspense( n1, n2, From 7e59a62ae3425774f3e2dd84bea50a2311d8f845 Mon Sep 17 00:00:00 2001 From: edison1105 Date: Tue, 9 Jan 2024 21:57:21 +0800 Subject: [PATCH 2/6] test: chore test case --- packages/runtime-core/__tests__/components/Suspense.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/runtime-core/__tests__/components/Suspense.spec.ts b/packages/runtime-core/__tests__/components/Suspense.spec.ts index 0141a22ca87..9ef9a5da23f 100644 --- a/packages/runtime-core/__tests__/components/Suspense.spec.ts +++ b/packages/runtime-core/__tests__/components/Suspense.spec.ts @@ -1524,7 +1524,7 @@ describe('Suspense', () => { }) //#8678 - test('avoid patch nested suspense with suspensible', async () => { + test('avoid patch nested suspense with suspensible if parent suspense is not resolved', async () => { const calls: string[] = [] let expected = '' From 96bec973f1c972785d71ec62c627c6c16ca2422f Mon Sep 17 00:00:00 2001 From: edison1105 Date: Tue, 9 Jan 2024 22:22:32 +0800 Subject: [PATCH 3/6] fix(Suspense): avoid unmont nested suspense with suspensible true if parent suspense is not resolved --- packages/runtime-core/src/components/Suspense.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/runtime-core/src/components/Suspense.ts b/packages/runtime-core/src/components/Suspense.ts index c3b66e4d813..6327344d755 100644 --- a/packages/runtime-core/src/components/Suspense.ts +++ b/packages/runtime-core/src/components/Suspense.ts @@ -720,6 +720,9 @@ function createSuspenseBoundary( }, unmount(parentSuspense, doRemove) { + if (isVNodeSuspensible(suspense.vnode) && parentSuspense?.deps !== 0) { + return + } suspense.isUnmounted = true if (suspense.activeBranch) { unmount( From ca3dd2e16c8ed89d1976e0c283408ea58e830ca9 Mon Sep 17 00:00:00 2001 From: daiwei Date: Wed, 10 Jan 2024 17:45:07 +0800 Subject: [PATCH 4/6] chore: improve code --- .../__tests__/components/Suspense.spec.ts | 260 +++++++++--------- .../runtime-core/src/components/Suspense.ts | 2 +- 2 files changed, 127 insertions(+), 135 deletions(-) diff --git a/packages/runtime-core/__tests__/components/Suspense.spec.ts b/packages/runtime-core/__tests__/components/Suspense.spec.ts index 9ef9a5da23f..1c0b8e98a49 100644 --- a/packages/runtime-core/__tests__/components/Suspense.spec.ts +++ b/packages/runtime-core/__tests__/components/Suspense.spec.ts @@ -1523,64 +1523,50 @@ describe('Suspense', () => { expect(calls).toContain('innerB mounted') }) - //#8678 - test('avoid patch nested suspense with suspensible if parent suspense is not resolved', async () => { + // #8206 + test('nested suspense with suspensible & no async deps', async () => { const calls: string[] = [] let expected = '' - const InnerA = defineAsyncComponent( - { - setup: () => { - calls.push('innerA created') - onMounted(() => { - calls.push('innerA mounted') - }) - return () => h('div', 'innerA') - }, + const InnerA = defineComponent({ + setup: () => { + calls.push('innerA created') + onMounted(() => { + calls.push('innerA mounted') + }) + return () => h('div', 'innerA') }, - 10, - ) + }) - const InnerB = defineAsyncComponent( - { - setup: () => { - calls.push('innerB created') - onMounted(() => { - calls.push('innerB mounted') - }) - return () => h('div', 'innerB') - }, + const InnerB = defineComponent({ + setup: () => { + calls.push('innerB created') + onMounted(() => { + calls.push('innerB mounted') + }) + return () => h('div', 'innerB') }, - 10, - ) + }) - const OuterA = defineAsyncComponent( - { - setup: (_, { slots }: any) => { - calls.push('outerA created') - onMounted(() => { - calls.push('outerA mounted') - }) - return () => - h(Fragment, null, [h('div', 'outerA'), slots.default?.()]) - }, + const OuterA = defineComponent({ + setup: (_, { slots }: any) => { + calls.push('outerA created') + onMounted(() => { + calls.push('outerA mounted') + }) + return () => h(Fragment, null, [h('div', 'outerA'), slots.default?.()]) }, - 5, - ) + }) - const OuterB = defineAsyncComponent( - { - setup: (_, { slots }: any) => { - calls.push('outerB created') - onMounted(() => { - calls.push('outerB mounted') - }) - return () => - h(Fragment, null, [h('div', 'outerB'), slots.default?.()]) - }, + const OuterB = defineComponent({ + setup: (_, { slots }: any) => { + calls.push('outerB created') + onMounted(() => { + calls.push('outerB mounted') + }) + return () => h(Fragment, null, [h('div', 'outerB'), slots.default?.()]) }, - 5, - ) + }) const outerToggle = ref(false) const innerToggle = ref(false) @@ -1594,7 +1580,7 @@ describe('Suspense', () => { * * */ - const Comp = { + const Comp = defineComponent({ setup() { return () => h(Suspense, null, { @@ -1613,103 +1599,105 @@ describe('Suspense', () => { fallback: h('div', 'fallback outer'), }) }, - } + }) - expected = `
fallback outer
` + expected = `
outerA
innerA
` const root = nodeOps.createElement('div') render(h(Comp), root) expect(serializeInner(root)).toBe(expected) - // mount outer component + // mount outer component and inner component await Promise.all(deps) await nextTick() expect(serializeInner(root)).toBe(expected) - expect(calls).toEqual([`outerA created`]) - - // mount inner component - await Promise.all(deps) - await nextTick() - expected = `
outerA
innerA
` - expect(serializeInner(root)).toBe(expected) - expect(calls).toEqual([ 'outerA created', 'innerA created', - 'outerA mounted', 'innerA mounted', + 'outerA mounted', ]) + // toggle outer component calls.length = 0 deps.length = 0 - - // toggle both outer and inner component outerToggle.value = true - innerToggle.value = true await nextTick() await Promise.all(deps) await nextTick() - expect(serializeInner(root)).toBe(expected) // expect not change + expected = `
outerB
innerA
` + expect(serializeInner(root)).toBe(expected) + expect(calls).toContain('outerB mounted') + expect(calls).toContain('innerA mounted') + // toggle inner component + calls.length = 0 + deps.length = 0 + innerToggle.value = true await Promise.all(deps) await nextTick() expected = `
outerB
innerB
` expect(serializeInner(root)).toBe(expected) - - // innerB only mount once - expect(calls).toEqual([ - 'outerB created', - 'innerB created', - 'outerB mounted', - 'innerB mounted', - ]) }) - // #8206 - test('nested suspense with suspensible & no async deps', async () => { + //#8678 + test('nested suspense (child suspense update before parent suspense resolve)', async () => { const calls: string[] = [] - let expected = '' - const InnerA = defineComponent({ - setup: () => { - calls.push('innerA created') - onMounted(() => { - calls.push('innerA mounted') - }) - return () => h('div', 'innerA') + const InnerA = defineAsyncComponent( + { + setup: () => { + calls.push('innerA created') + onMounted(() => { + calls.push('innerA mounted') + }) + return () => h('div', 'innerA') + }, }, - }) + 10, + ) - const InnerB = defineComponent({ - setup: () => { - calls.push('innerB created') - onMounted(() => { - calls.push('innerB mounted') - }) - return () => h('div', 'innerB') + const InnerB = defineAsyncComponent( + { + setup: () => { + calls.push('innerB created') + onMounted(() => { + calls.push('innerB mounted') + }) + return () => h('div', 'innerB') + }, }, - }) + 10, + ) - const OuterA = defineComponent({ - setup: (_, { slots }: any) => { - calls.push('outerA created') - onMounted(() => { - calls.push('outerA mounted') - }) - return () => h(Fragment, null, [h('div', 'outerA'), slots.default?.()]) + const OuterA = defineAsyncComponent( + { + setup: (_, { slots }: any) => { + calls.push('outerA created') + onMounted(() => { + calls.push('outerA mounted') + }) + return () => + h(Fragment, null, [h('div', 'outerA'), slots.default?.()]) + }, }, - }) + 5, + ) - const OuterB = defineComponent({ - setup: (_, { slots }: any) => { - calls.push('outerB created') - onMounted(() => { - calls.push('outerB mounted') - }) - return () => h(Fragment, null, [h('div', 'outerB'), slots.default?.()]) + const OuterB = defineAsyncComponent( + { + setup: (_, { slots }: any) => { + calls.push('outerB created') + onMounted(() => { + calls.push('outerB mounted') + }) + return () => + h(Fragment, null, [h('div', 'outerB'), slots.default?.()]) + }, }, - }) + 5, + ) const outerToggle = ref(false) const innerToggle = ref(false) @@ -1717,71 +1705,75 @@ describe('Suspense', () => { /** * * - * + * * * * * */ - const Comp = defineComponent({ + const Comp = { setup() { return () => h(Suspense, null, { default: [ h(outerToggle.value ? OuterB : OuterA, null, { default: () => - h( - Suspense, - { suspensible: true }, - { - default: h(innerToggle.value ? InnerB : InnerA), - }, - ), + h(Suspense, null, { + default: h(innerToggle.value ? InnerB : InnerA), + }), }), ], fallback: h('div', 'fallback outer'), }) }, - }) + } - expected = `
outerA
innerA
` const root = nodeOps.createElement('div') render(h(Comp), root) - expect(serializeInner(root)).toBe(expected) + expect(serializeInner(root)).toBe(`
fallback outer
`) - // mount outer component and inner component + // mount outer component await Promise.all(deps) await nextTick() - expect(serializeInner(root)).toBe(expected) + expect(serializeInner(root)).toBe(`
outerA
`) + expect(calls).toEqual([`outerA created`, `outerA mounted`]) + + // mount inner component + await Promise.all(deps) + await nextTick() + expect(serializeInner(root)).toBe(`
outerA
innerA
`) + expect(calls).toEqual([ 'outerA created', + 'outerA mounted', 'innerA created', 'innerA mounted', - 'outerA mounted', ]) - // toggle outer component calls.length = 0 deps.length = 0 + + // toggle both outer and inner components outerToggle.value = true + innerToggle.value = true await nextTick() await Promise.all(deps) await nextTick() - expected = `
outerB
innerA
` - expect(serializeInner(root)).toBe(expected) - expect(calls).toContain('outerB mounted') - expect(calls).toContain('innerA mounted') + expect(serializeInner(root)).toBe(`
outerB
`) - // toggle inner component - calls.length = 0 - deps.length = 0 - innerToggle.value = true await Promise.all(deps) await nextTick() - expected = `
outerB
innerB
` - expect(serializeInner(root)).toBe(expected) + expect(serializeInner(root)).toBe(`
outerB
innerB
`) + + // innerB only mount once + expect(calls).toEqual([ + 'outerB created', + 'outerB mounted', + 'innerB created', + 'innerB mounted', + ]) }) // #6416 diff --git a/packages/runtime-core/src/components/Suspense.ts b/packages/runtime-core/src/components/Suspense.ts index 6327344d755..46fe7397835 100644 --- a/packages/runtime-core/src/components/Suspense.ts +++ b/packages/runtime-core/src/components/Suspense.ts @@ -91,7 +91,7 @@ export const SuspenseImpl = { rendererInternals, ) } else { - if (isVNodeSuspensible(n2) && parentSuspense?.deps !== 0) { + if (parentSuspense && parentSuspense.deps > 0) { n2.suspense = n1.suspense return } From 7f19cd0b350e3229058a9c2fb6b022723785547c Mon Sep 17 00:00:00 2001 From: daiwei Date: Wed, 10 Jan 2024 17:49:54 +0800 Subject: [PATCH 5/6] revert --- packages/runtime-core/src/components/Suspense.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/runtime-core/src/components/Suspense.ts b/packages/runtime-core/src/components/Suspense.ts index 46fe7397835..7e4df56f3b3 100644 --- a/packages/runtime-core/src/components/Suspense.ts +++ b/packages/runtime-core/src/components/Suspense.ts @@ -720,9 +720,6 @@ function createSuspenseBoundary( }, unmount(parentSuspense, doRemove) { - if (isVNodeSuspensible(suspense.vnode) && parentSuspense?.deps !== 0) { - return - } suspense.isUnmounted = true if (suspense.activeBranch) { unmount( From 3d9ff3403d561503d964d304887153750ad2b8bf Mon Sep 17 00:00:00 2001 From: daiwei Date: Thu, 11 Jan 2024 17:23:03 +0800 Subject: [PATCH 6/6] chore: add comments --- packages/runtime-core/src/components/Suspense.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/runtime-core/src/components/Suspense.ts b/packages/runtime-core/src/components/Suspense.ts index 46493fae810..b9e0ccdd710 100644 --- a/packages/runtime-core/src/components/Suspense.ts +++ b/packages/runtime-core/src/components/Suspense.ts @@ -91,6 +91,14 @@ export const SuspenseImpl = { rendererInternals, ) } else { + // #8678 if the current suspense needs to be patched and parentSuspense has + // not been resolved. this means that both the current suspense and parentSuspense + // need to be patched. because parentSuspense's pendingBranch includes the + // current suspense, it will be processed twice: + // 1. current patch + // 2. mounting along with the pendingBranch of parentSuspense + // it is necessary to skip the current patch to avoid multiple mounts + // of inner components. if (parentSuspense && parentSuspense.deps > 0) { n2.suspense = n1.suspense return