refactor: simplify suspense ssr + adjust behavior

2020-03-16 15:38:35 -04:00 · 2020-03-16 15:38:35 -04:00 · 38f0269765
commit 38f0269765
parent eb5b32fead
7 changed files with 131 additions and 223 deletions
--- 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
-        }))
+        })
      }"
    `)
  })
--- 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
    ])
  )
 }
--- 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()
    })
  })

--- 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 () => {
--- 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()
  })
 })
--- 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(`<!---->`)
  }
 }
--- 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
-    }
-  }
-}