From 38f0269765cfd5a01d07b131e1a3d9475eb462d8 Mon Sep 17 00:00:00 2001
From: Evan You <yyx990803@gmail.com>
Date: Mon, 16 Mar 2020 15:38:35 -0400
Subject: [PATCH] refactor: simplify suspense ssr + adjust behavior

---
 .../__tests__/ssrSuspense.spec.ts             |  14 +-
 .../src/transforms/ssrTransformSuspense.ts    |   7 +-
 .../runtime-core/__tests__/component.spec.ts  |   4 +
 .../__tests__/renderToString.spec.ts          |   4 +-
 .../__tests__/ssrSuspense.spec.ts             | 232 +++++++-----------
 .../src/helpers/ssrRenderSuspense.ts          |  38 +--
 .../server-renderer/src/renderToString.ts     |  55 ++---
 7 files changed, 131 insertions(+), 223 deletions(-)

diff --git a/packages/compiler-ssr/__tests__/ssrSuspense.spec.ts b/packages/compiler-ssr/__tests__/ssrSuspense.spec.ts
index 83a75cf91..db60f5e23 100644
--- a/packages/compiler-ssr/__tests__/ssrSuspense.spec.ts
+++ b/packages/compiler-ssr/__tests__/ssrSuspense.spec.ts
@@ -9,12 +9,12 @@ describe('ssr compile: suspense', () => {
       return function ssrRender(_ctx, _push, _parent) {
         const _component_foo = _resolveComponent(\\"foo\\")
 
-        _push(_ssrRenderSuspense({
-          default: (_push) => {
+        _ssrRenderSuspense(_push, {
+          default: () => {
             _push(_ssrRenderComponent(_component_foo, null, null, _parent))
           },
           _: 1
-        }))
+        })
       }"
     `)
   })
@@ -36,15 +36,15 @@ describe('ssr compile: suspense', () => {
       return function ssrRender(_ctx, _push, _parent) {
         const _component_foo = _resolveComponent(\\"foo\\")
 
-        _push(_ssrRenderSuspense({
-          default: (_push) => {
+        _ssrRenderSuspense(_push, {
+          default: () => {
             _push(_ssrRenderComponent(_component_foo, null, null, _parent))
           },
-          fallback: (_push) => {
+          fallback: () => {
             _push(\` loading... \`)
           },
           _: 1
-        }))
+        })
       }"
     `)
   })
diff --git a/packages/compiler-ssr/src/transforms/ssrTransformSuspense.ts b/packages/compiler-ssr/src/transforms/ssrTransformSuspense.ts
index 2a948d6fa..c590eabea 100644
--- a/packages/compiler-ssr/src/transforms/ssrTransformSuspense.ts
+++ b/packages/compiler-ssr/src/transforms/ssrTransformSuspense.ts
@@ -38,7 +38,7 @@ export function ssrTransformSuspense(
       wipMap.set(node, wipEntry)
       wipEntry.slotsExp = buildSlots(node, context, (_props, children, loc) => {
         const fn = createFunctionExpression(
-          [`_push`],
+          [],
           undefined, // no return, assign body later
           true, // newline
           false, // suspense slots are not treated as normal slots
@@ -71,8 +71,9 @@ export function ssrProcessSuspense(
   }
   // _push(ssrRenderSuspense(slots))
   context.pushStatement(
-    createCallExpression(`_push`, [
-      createCallExpression(context.helper(SSR_RENDER_SUSPENSE), [slotsExp])
+    createCallExpression(context.helper(SSR_RENDER_SUSPENSE), [
+      `_push`,
+      slotsExp
     ])
   )
 }
diff --git a/packages/runtime-core/__tests__/component.spec.ts b/packages/runtime-core/__tests__/component.spec.ts
index dad6f8ca8..4fd8707b6 100644
--- a/packages/runtime-core/__tests__/component.spec.ts
+++ b/packages/runtime-core/__tests__/component.spec.ts
@@ -6,8 +6,11 @@ import {
   nextTick,
   defineComponent
 } from '@vue/runtime-test'
+import { mockWarn } from '@vue/shared'
 
 describe('renderer: component', () => {
+  mockWarn()
+
   test.todo('should work')
 
   test.todo('shouldUpdateComponent')
@@ -40,6 +43,7 @@ describe('renderer: component', () => {
       expect(b1).toBe(true)
       expect(b2).toBe(true)
       expect(b3).toBe('')
+      expect('type check failed for prop "b1"').toHaveBeenWarned()
     })
   })
 
diff --git a/packages/server-renderer/__tests__/renderToString.spec.ts b/packages/server-renderer/__tests__/renderToString.spec.ts
index 7a0de6fe9..2a3ab7e00 100644
--- a/packages/server-renderer/__tests__/renderToString.spec.ts
+++ b/packages/server-renderer/__tests__/renderToString.spec.ts
@@ -8,11 +8,11 @@ import {
   ref,
   defineComponent
 } from 'vue'
-import { escapeHtml, mockError } from '@vue/shared'
+import { escapeHtml, mockWarn } from '@vue/shared'
 import { renderToString, renderComponent } from '../src/renderToString'
 import { ssrRenderSlot } from '../src/helpers/ssrRenderSlot'
 
-mockError()
+mockWarn()
 
 describe('ssr: renderToString', () => {
   test('should apply app context', async () => {
diff --git a/packages/server-renderer/__tests__/ssrSuspense.spec.ts b/packages/server-renderer/__tests__/ssrSuspense.spec.ts
index e7bd3b5ba..89009ac1a 100644
--- a/packages/server-renderer/__tests__/ssrSuspense.spec.ts
+++ b/packages/server-renderer/__tests__/ssrSuspense.spec.ts
@@ -1,11 +1,9 @@
 import { createApp, h, Suspense } from 'vue'
 import { renderToString } from '../src/renderToString'
-import { ssrRenderSuspense } from '../src/helpers/ssrRenderSuspense'
-import { ssrRenderComponent } from '../src'
-import { mockError } from '@vue/shared'
+import { mockWarn } from '@vue/shared'
 
 describe('SSR Suspense', () => {
-  mockError()
+  mockWarn()
 
   const ResolvingAsync = {
     async setup() {
@@ -19,161 +17,109 @@ describe('SSR Suspense', () => {
     }
   }
 
-  describe('compiled', () => {
-    test('basic', async () => {
-      const app = createApp({
-        ssrRender(_ctx, _push) {
-          _push(
-            ssrRenderSuspense({
-              default: _push => {
-                _push('<div>async</div>')
-              }
-            })
-          )
-        }
-      })
+  test('content', async () => {
+    const Comp = {
+      render() {
+        return h(Suspense, null, {
+          default: h(ResolvingAsync),
+          fallback: h('div', 'fallback')
+        })
+      }
+    }
 
-      expect(await renderToString(app)).toBe(`<!--[--><div>async</div><!--]-->`)
-    })
-
-    test('with async component', async () => {
-      const app = createApp({
-        ssrRender(_ctx, _push) {
-          _push(
-            ssrRenderSuspense({
-              default: _push => {
-                _push(ssrRenderComponent(ResolvingAsync))
-              }
-            })
-          )
-        }
-      })
-
-      expect(await renderToString(app)).toBe(`<!--[--><div>async</div><!--]-->`)
-    })
-
-    test('fallback', async () => {
-      const app = createApp({
-        ssrRender(_ctx, _push) {
-          _push(
-            ssrRenderSuspense({
-              default: _push => {
-                _push(ssrRenderComponent(RejectingAsync))
-              },
-              fallback: _push => {
-                _push('<div>fallback</div>')
-              }
-            })
-          )
-        }
-      })
-
-      expect(await renderToString(app)).toBe(
-        `<!--[--><div>fallback</div><!--]-->`
-      )
-      expect('Uncaught error in async setup').toHaveBeenWarned()
-    })
+    expect(await renderToString(createApp(Comp))).toBe(`<div>async</div>`)
   })
 
-  describe('vnode', () => {
-    test('content', async () => {
-      const Comp = {
-        render() {
-          return h(Suspense, null, {
-            default: h(ResolvingAsync),
-            fallback: h('div', 'fallback')
-          })
-        }
+  test('reject', async () => {
+    const Comp = {
+      render() {
+        return h(Suspense, null, {
+          default: h(RejectingAsync),
+          fallback: h('div', 'fallback')
+        })
       }
+    }
 
-      expect(await renderToString(createApp(Comp))).toBe(`<div>async</div>`)
-    })
+    expect(await renderToString(createApp(Comp))).toBe(`<!---->`)
+    expect('Uncaught error in async setup').toHaveBeenWarned()
+    expect('missing template').toHaveBeenWarned()
+  })
 
-    test('fallback', async () => {
-      const Comp = {
-        render() {
-          return h(Suspense, null, {
-            default: h(RejectingAsync),
-            fallback: h('div', 'fallback')
-          })
-        }
+  test('2 components', async () => {
+    const Comp = {
+      render() {
+        return h(Suspense, null, {
+          default: h('div', [h(ResolvingAsync), h(ResolvingAsync)]),
+          fallback: h('div', 'fallback')
+        })
       }
+    }
 
-      expect(await renderToString(createApp(Comp))).toBe(`<div>fallback</div>`)
-      expect('Uncaught error in async setup').toHaveBeenWarned()
-    })
+    expect(await renderToString(createApp(Comp))).toBe(
+      `<div><div>async</div><div>async</div></div>`
+    )
+  })
 
-    test('2 components', async () => {
-      const Comp = {
-        render() {
-          return h(Suspense, null, {
-            default: h('div', [h(ResolvingAsync), h(ResolvingAsync)]),
-            fallback: h('div', 'fallback')
-          })
-        }
+  test('resolving component + rejecting component', async () => {
+    const Comp = {
+      render() {
+        return h(Suspense, null, {
+          default: h('div', [h(ResolvingAsync), h(RejectingAsync)]),
+          fallback: h('div', 'fallback')
+        })
       }
+    }
 
-      expect(await renderToString(createApp(Comp))).toBe(
-        `<div><div>async</div><div>async</div></div>`
-      )
-    })
+    expect(await renderToString(createApp(Comp))).toBe(
+      `<div><div>async</div><!----></div>`
+    )
+    expect('Uncaught error in async setup').toHaveBeenWarned()
+    expect('missing template or render function').toHaveBeenWarned()
+  })
 
-    test('resolving component + rejecting component', async () => {
-      const Comp = {
-        render() {
-          return h(Suspense, null, {
-            default: h('div', [h(ResolvingAsync), h(RejectingAsync)]),
-            fallback: h('div', 'fallback')
-          })
-        }
+  test('failing suspense in passing suspense', async () => {
+    const Comp = {
+      render() {
+        return h(Suspense, null, {
+          default: h('div', [
+            h(ResolvingAsync),
+            h(Suspense, null, {
+              default: h('div', [h(RejectingAsync)]),
+              fallback: h('div', 'fallback 2')
+            })
+          ]),
+          fallback: h('div', 'fallback 1')
+        })
       }
+    }
 
-      expect(await renderToString(createApp(Comp))).toBe(`<div>fallback</div>`)
-      expect('Uncaught error in async setup').toHaveBeenWarned()
-    })
+    expect(await renderToString(createApp(Comp))).toBe(
+      `<div><div>async</div><div><!----></div></div>`
+    )
+    expect('Uncaught error in async setup').toHaveBeenWarned()
+    expect('missing template').toHaveBeenWarned()
+  })
 
-    test('failing suspense in passing suspense', async () => {
-      const Comp = {
-        render() {
-          return h(Suspense, null, {
-            default: h('div', [
-              h(ResolvingAsync),
-              h(Suspense, null, {
-                default: h('div', [h(RejectingAsync)]),
-                fallback: h('div', 'fallback 2')
-              })
-            ]),
-            fallback: h('div', 'fallback 1')
-          })
-        }
+  test('passing suspense in failing suspense', async () => {
+    const Comp = {
+      render() {
+        return h(Suspense, null, {
+          default: h('div', [
+            h(RejectingAsync),
+            h(Suspense, null, {
+              default: h('div', [h(ResolvingAsync)]),
+              fallback: h('div', 'fallback 2')
+            })
+          ]),
+          fallback: h('div', 'fallback 1')
+        })
       }
+    }
 
-      expect(await renderToString(createApp(Comp))).toBe(
-        `<div><div>async</div><div>fallback 2</div></div>`
-      )
-      expect('Uncaught error in async setup').toHaveBeenWarned()
-    })
-
-    test('passing suspense in failing suspense', async () => {
-      const Comp = {
-        render() {
-          return h(Suspense, null, {
-            default: h('div', [
-              h(RejectingAsync),
-              h(Suspense, null, {
-                default: h('div', [h(ResolvingAsync)]),
-                fallback: h('div', 'fallback 2')
-              })
-            ]),
-            fallback: h('div', 'fallback 1')
-          })
-        }
-      }
-
-      expect(await renderToString(createApp(Comp))).toBe(
-        `<div>fallback 1</div>`
-      )
-      expect('Uncaught error in async setup').toHaveBeenWarned()
-    })
+    expect(await renderToString(createApp(Comp))).toBe(
+      `<div><!----><div><div>async</div></div></div>`
+    )
+    expect('Uncaught error in async setup').toHaveBeenWarned()
+    expect('missing template').toHaveBeenWarned()
   })
 })
diff --git a/packages/server-renderer/src/helpers/ssrRenderSuspense.ts b/packages/server-renderer/src/helpers/ssrRenderSuspense.ts
index 00546a687..97586988c 100644
--- a/packages/server-renderer/src/helpers/ssrRenderSuspense.ts
+++ b/packages/server-renderer/src/helpers/ssrRenderSuspense.ts
@@ -1,30 +1,14 @@
-import { PushFn, ResolvedSSRBuffer, createBuffer } from '../renderToString'
+import { PushFn } from '../renderToString'
 
-type ContentRenderFn = (push: PushFn) => void
-
-export async function ssrRenderSuspense({
-  default: renderContent,
-  fallback: renderFallback
-}: Record<string, ContentRenderFn | undefined>): Promise<ResolvedSSRBuffer> {
-  try {
-    if (renderContent) {
-      const { push, getBuffer } = createBuffer()
-      push(`<!--[-->`)
-      renderContent(push)
-      push(`<!--]-->`)
-      return await getBuffer()
-    } else {
-      return []
-    }
-  } catch {
-    if (renderFallback) {
-      const { push, getBuffer } = createBuffer()
-      push(`<!--[-->`)
-      renderFallback(push)
-      push(`<!--]-->`)
-      return getBuffer()
-    } else {
-      return []
-    }
+export async function ssrRenderSuspense(
+  push: PushFn,
+  { default: renderContent }: Record<string, (() => void) | undefined>
+) {
+  if (renderContent) {
+    push(`<!--[-->`)
+    renderContent()
+    push(`<!--]-->`)
+  } else {
+    push(`<!---->`)
   }
 }
diff --git a/packages/server-renderer/src/renderToString.ts b/packages/server-renderer/src/renderToString.ts
index 09a5ad3b0..88438efb0 100644
--- a/packages/server-renderer/src/renderToString.ts
+++ b/packages/server-renderer/src/renderToString.ts
@@ -11,7 +11,8 @@ import {
   ssrUtils,
   Slots,
   createApp,
-  ssrContextKey
+  ssrContextKey,
+  warn
 } from 'vue'
 import {
   ShapeFlags,
@@ -138,8 +139,6 @@ export function renderComponent(
   )
 }
 
-export const AsyncSetupErrorMarker = Symbol('Vue async setup error')
-
 function renderComponentVNode(
   vnode: VNode,
   parentComponent: ComponentInternalInstance | null = null
@@ -153,17 +152,7 @@ function renderComponentVNode(
   if (isPromise(res)) {
     return res
       .catch(err => {
-        // normalize async setup rejection
-        if (!(err instanceof Error)) {
-          err = new Error(String(err))
-        }
-        err[AsyncSetupErrorMarker] = true
-        console.error(
-          `[@vue/server-renderer]: Uncaught error in async setup:\n`,
-          err
-        )
-        // rethrow for suspense
-        throw err
+        warn(`[@vue/server-renderer]: Uncaught error in async setup:\n`, err)
       })
       .then(() => renderComponentSubTree(instance))
   } else {
@@ -192,11 +181,12 @@ function renderComponentSubTree(
     } else if (instance.render) {
       renderVNode(push, renderComponentRoot(instance), instance)
     } else {
-      throw new Error(
+      warn(
         `Component ${
           comp.name ? `${comp.name} ` : ``
         } is missing template or render function.`
       )
+      push(`<!---->`)
     }
   }
   return getBuffer()
@@ -233,7 +223,7 @@ function ssrCompile(
             err.loc.start.offset,
             err.loc.end.offset
           )
-        console.error(codeFrame ? `${message}\n${codeFrame}` : message)
+        warn(codeFrame ? `${message}\n${codeFrame}` : message)
       } else {
         throw err
       }
@@ -268,9 +258,13 @@ function renderVNode(
       } else if (shapeFlag & ShapeFlags.PORTAL) {
         renderPortalVNode(vnode, parentComponent)
       } else if (shapeFlag & ShapeFlags.SUSPENSE) {
-        push(renderSuspenseVNode(vnode, parentComponent))
+        renderVNode(
+          push,
+          normalizeSuspenseChildren(vnode).content,
+          parentComponent
+        )
       } else {
-        console.error(
+        warn(
           '[@vue/server-renderer] Invalid VNode type:',
           type,
           `(${typeof type})`
@@ -350,11 +344,11 @@ function renderPortalVNode(
 ) {
   const target = vnode.props && vnode.props.target
   if (!target) {
-    console.error(`[@vue/server-renderer] Portal is missing target prop.`)
+    warn(`[@vue/server-renderer] Portal is missing target prop.`)
     return []
   }
   if (!isString(target)) {
-    console.error(
+    warn(
       `[@vue/server-renderer] Portal target must be a query selector string.`
     )
     return []
@@ -385,24 +379,3 @@ async function resolvePortals(context: SSRContext) {
     }
   }
 }
-
-async function renderSuspenseVNode(
-  vnode: VNode,
-  parentComponent: ComponentInternalInstance
-): Promise<ResolvedSSRBuffer> {
-  const { content, fallback } = normalizeSuspenseChildren(vnode)
-  try {
-    const { push, getBuffer } = createBuffer()
-    renderVNode(push, content, parentComponent)
-    // await here so error can be caught
-    return await getBuffer()
-  } catch (e) {
-    if (e[AsyncSetupErrorMarker]) {
-      const { push, getBuffer } = createBuffer()
-      renderVNode(push, fallback, parentComponent)
-      return getBuffer()
-    } else {
-      throw e
-    }
-  }
-}