From: Eduardo San Martin Morote <posva13@gmail.com>
Date: Wed, 9 Feb 2022 16:35:00 +0000 (+0100)
Subject: refactor: wip proper promise chain even with redirects
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=71893682b8b3b8e78d80fcf5971a5ae47e07e82c;p=thirdparty%2Fvuejs%2Frouter.git

refactor: wip proper promise chain even with redirects
---

diff --git a/e2e/suspense-view/index.ts b/e2e/suspense-view/index.ts
index c9e88792..1cbbb407 100644
--- a/e2e/suspense-view/index.ts
+++ b/e2e/suspense-view/index.ts
@@ -31,11 +31,11 @@ const delay = (t: number) => new Promise(r => setTimeout(r, t))
 
 const AsyncImport = defineAsyncComponent(async () => {
   await delay(1000)
-  console.log('finished loading async component')
+  log('finished loading async component')
   return defineComponent({
     name: 'AsyncImport',
     beforeMount() {
-      console.log('done')
+      log('AsyncImport done')
     },
     template: `<div>AsyncImport</div>`,
   })
@@ -47,27 +47,50 @@ setInterval(() => {
   n.value++
 }, 1000)
 
+function log(...args: any[]) {
+  const now = new Date()
+  const time = `${now.getHours().toString().padStart(2, '0')}:${now
+    .getMinutes()
+    .toString()
+    .padStart(2, '0')}:${now.getSeconds().toString().padStart(2, '0')}.${now
+    .getMilliseconds()
+    .toString()
+    .padStart(3, '0')}`
+  console.log(`[${time}]`, ...args)
+}
+
 /**
  * creates a component that logs the guards
  * @param name
  */
-function createAsyncComponent(key: string, isAsync = true) {
+function createAsyncComponent(
+  key: string,
+  isAsync = true,
+  hasAsyncChild = false
+) {
   return defineComponent({
     name: key,
-    components: { AsyncImport },
-    template: `<div id="${key}">${key}: n = {{n}}.<AsyncImport v-if="${isAsync}" /></div>`,
+    components: {
+      AsyncImport,
+      AsyncChild: hasAsyncChild
+        ? createAsyncComponent(key + ':child', true, false)
+        : null,
+    },
+    template: `<div id="${key}">${key}: n = {{n}}.<AsyncImport v-if="${isAsync}" />
+    ${hasAsyncChild ? `<AsyncChild />` : ''}
+    </div>`,
 
     setup() {
       const route = useRoute()
       const shouldFail = !!route.query.fail
 
-      console.log(`Setup of ${key}...`)
+      log(`Setup of ${key}...`)
 
       const ret = { n }
 
       return isAsync
         ? delay(2000).then(() => {
-            console.log(`finished setup of ${key}`)
+            log(`finished setup of ${key}`)
 
             return shouldFail ? Promise.reject(new Error('failed')) : ret
           })
@@ -82,7 +105,7 @@ function createAsyncNestedComponent(key: string) {
     template: `<div id="${key}">${key}:
     <SusRouterView @pending="log('⏳ (nested ${key}) pending', $event)" @resolve="log('✅ (nested ${key}) resolve', $event)">
       <template #fallback>
-        Loading...
+        ${key} loading...
       </template>
       <template v-slot="{ Component }">
         <component :is="Component" class="view" />
@@ -94,7 +117,7 @@ function createAsyncNestedComponent(key: string) {
       const route = useRoute()
       const shouldFail = !!route.query.fail
 
-      console.log(`Setup of ${key}...`)
+      log(`Setup of ${key}...`)
 
       return delay(100).then(() =>
         shouldFail ? Promise.reject(new Error('failed')) : {}
@@ -117,6 +140,10 @@ const router = createRouter({
       path: '/foo',
       component: Foo,
     },
+    {
+      path: '/foo-with-child',
+      component: createAsyncComponent('WithChild', true, true),
+    },
     {
       path: '/foo-async',
       component: FooAsync,
@@ -155,6 +182,7 @@ route: {{ $route.fullPath }}
     <ul>
       <li><router-link to="/">/</router-link></li>
       <li><router-link to="/foo">/foo</router-link></li>
+      <li><router-link to="/foo-with-child" v-slot="{ route }">{{ route.fullPath }}</router-link></li>
       <li><router-link to="/foo-async">/foo-async</router-link></li>
       <li><router-link id="update-query" :to="{ query: { n: (Number($route.query.n) || 0) + 1 }}" v-slot="{ route }">{{ route.fullPath }}</router-link></li>
       <li><router-link to="/nested/foo">Nested with sync child</router-link></li>
@@ -163,43 +191,51 @@ route: {{ $route.fullPath }}
       <li><router-link to="/nested-async/foo-async">Nested async with async child</router-link></li>
     </ul>
 
-    <SusRouterView @pending="log('⏳ pending', $event)" @resolve="log('✅ resolve', $event)">
+    <SusRouterView
+      @fallback="log('⌛️ fallback', $event)"
+      @pending="log('⏳ pending', $event)"
+      @resolve="log('✅ resolve', $event)"
+      :timeout="800"
+    >
       <template #fallback>
-        Loading...
-      </template>
-      <template v-slot="{ Component }">
-        <component :is="Component" class="view" />
+        Root Loading...
       </template>
     </SusRouterView>
   `,
   setup() {
     onErrorCaptured(err => {
-      console.log('❌ From Suspense', err)
+      log('❌ From Suspense', err)
     })
     return { clear: console.clear, shouldFail }
   },
 })
 app.component('SusRouterView', SusRouterView)
-app.config.globalProperties.log = console.log
+app.config.globalProperties.log = log
 
-router.beforeEach((to, from) => {
+router.beforeEach(async (to, from) => {
   console.log('-'.repeat(10))
-  console.log(`🏎 ${from.fullPath} -> ${to.fullPath}`)
+  log(`🏎 ${from.fullPath} -> ${to.fullPath}`)
+  const wait = Number(to.query.wait) || 0
+  if (wait) {
+    log(`⏲ waiting ${wait}ms`)
+    await delay(wait)
+    log(`⏲ 🔔 Done Waiting ${wait}ms`)
+  }
   if (shouldFail.value && !to.query.fail)
     return { ...to, query: { ...to.query, fail: 'yes' } }
   return
 })
 router.afterEach((to, from, failure) => {
   if (failure) {
-    console.log(`🛑 ${from.fullPath} -> ${to.fullPath}`)
+    log(`🛑 ${from.fullPath} -> ${to.fullPath}`)
   } else {
-    console.log(`🏁 ${from.fullPath} -> ${to.fullPath}`)
+    log(`🏁 ${from.fullPath} -> ${to.fullPath}`)
   }
 })
 router.onError((error, to, from) => {
-  console.log(`💥 ${from.fullPath} -> ${to.fullPath}`)
+  log(`💥 ${from.fullPath} -> ${to.fullPath}`)
   console.error(error)
-  console.log('-'.repeat(10))
+  log('-'.repeat(10))
 })
 app.use(router)
 
diff --git a/e2e/suspense/notes.md b/e2e/suspense/notes.md
index 2039c4c8..0f93a547 100644
--- a/e2e/suspense/notes.md
+++ b/e2e/suspense/notes.md
@@ -138,7 +138,11 @@ import { getUser } from './api'
  */
 const user = ref()
 
+// +--- Allows blocking the initial navigation
+// |
+// v
 await onBeforeNavigation(async (to, from) => {
+  // this will be executed on any navigation except when leaving (we can have a different hook like `onBeforeEach()`)
   user.value = await getUser(to.params.id)
 })
 </script>
diff --git a/e2e/tsconfig.json b/e2e/tsconfig.json
index d61bd724..a711ec10 100644
--- a/e2e/tsconfig.json
+++ b/e2e/tsconfig.json
@@ -1,7 +1,7 @@
 {
   "include": ["index.ts", "*/*.ts", "../src/global.d.ts"],
   "compilerOptions": {
-    "target": "es6",
+    "target": "ESNext",
     "module": "commonjs",
     // "lib": ["es2017.object"] /* Specify library files to be included in the compilation. */,
     "declaration": true,
diff --git a/src/SusRouterView.ts b/src/SusRouterView.ts
index 42e87530..2f0dd81c 100644
--- a/src/SusRouterView.ts
+++ b/src/SusRouterView.ts
@@ -51,9 +51,10 @@ export const SusRouterViewImpl = /*#__PURE__*/ defineComponent({
       type: String as PropType<string>,
       default: 'default',
     },
+    timeout: Number,
     route: Object as PropType<RouteLocationNormalizedLoaded>,
   },
-  emits: ['resolve', 'pending'],
+  emits: ['resolve', 'pending', 'fallback'],
 
   setup(props, { attrs, slots, emit }) {
     __DEV__ && warnDeprecatedUsage()
@@ -152,6 +153,7 @@ export const SusRouterViewImpl = /*#__PURE__*/ defineComponent({
       const component = h(
         Suspense,
         {
+          timeout: props.timeout,
           onPending: () => {
             unregisterPendingView = addPendingView(Symbol())
             emit('pending', String(ViewComponent.name || 'unnamed'))
@@ -160,7 +162,9 @@ export const SusRouterViewImpl = /*#__PURE__*/ defineComponent({
             unregisterPendingView && unregisterPendingView()
             emit('resolve', String(ViewComponent.name || 'unnamed'))
           },
-          // onResolve,
+          onFallback: () => {
+            emit('fallback', String(ViewComponent.name || 'unnamed'))
+          },
         },
         {
           fallback: slots.fallback,
diff --git a/src/router.ts b/src/router.ts
index 16dfd3d0..f3341ca4 100644
--- a/src/router.ts
+++ b/src/router.ts
@@ -704,90 +704,93 @@ export function createRouter(options: RouterOptions): Router {
       )
     }
 
+    // reset any pending views
     pendingViews.clear()
 
-    return (pendingNavigation.value = new Promise(
-      (promiseResolve, promiseReject) => {
-        rejectPendingNavigation = promiseReject
-
-        return (failure ? Promise.resolve(failure) : navigate(toLocation, from))
-          .catch((error: NavigationFailure | NavigationRedirectError) =>
-            isNavigationFailure(error)
-              ? error
-              : // reject any unknown error
-                triggerError(error, toLocation, from)
-          )
-          .then(
-            (failure: NavigationFailure | NavigationRedirectError | void) => {
-              if (failure) {
-                if (
-                  isNavigationFailure(
-                    failure,
-                    ErrorTypes.NAVIGATION_GUARD_REDIRECT
-                  )
-                ) {
-                  if (
-                    __DEV__ &&
-                    // we are redirecting to the same location we were already at
-                    isSameRouteLocation(
-                      stringifyQuery,
-                      resolve(failure.to),
-                      toLocation
-                    ) &&
-                    // and we have done it a couple of times
-                    redirectedFrom &&
-                    // @ts-expect-error: added only in dev
-                    (redirectedFrom._count = redirectedFrom._count
-                      ? // @ts-expect-error
-                        redirectedFrom._count + 1
-                      : 1) > 10
-                  ) {
-                    warn(
-                      `Detected an infinite redirection in a navigation guard when going from "${from.fullPath}" to "${toLocation.fullPath}". Aborting to avoid a Stack Overflow. This will break in production if not fixed.`
-                    )
-                    return Promise.reject(
-                      new Error('Infinite redirect in navigation guard')
-                    )
-                  }
-
-                  // FIXME: find a way to keep the return pattern of promises to handle reusing the promise maybe by
-                  // passing the resolve, reject as parameters to pushWithRedirect
-
-                  return pushWithRedirect(
-                    // keep options
-                    assign(locationAsObject(failure.to), {
-                      state: data,
-                      force,
-                      replace,
-                    }),
-                    // preserve the original redirectedFrom if any
-                    redirectedFrom || toLocation
-                  )
-                }
-              } else {
-                // if we fail we don't finalize the navigation
-                pendingNavigation.value = null
-                failure = finalizeNavigation(
-                  toLocation as RouteLocationNormalizedLoaded,
-                  from,
-                  true,
-                  replace,
-                  data
-                )
-              }
-              resolvePendingNavigation = () => {
-                triggerAfterEach(
-                  toLocation as RouteLocationNormalizedLoaded,
-                  from,
-                  failure as any
-                )
-                promiseResolve(failure as any)
-              }
-              return failure
+    return (pendingNavigation.value = (
+      failure ? Promise.resolve(failure) : navigate(toLocation, from)
+    )
+      .catch(
+        (
+          errorOrNavigationFailure: NavigationFailure | NavigationRedirectError
+        ) =>
+          isNavigationFailure(errorOrNavigationFailure)
+            ? errorOrNavigationFailure
+            : // triggerError returns a rejected promise to avoid the next then()
+              triggerError(errorOrNavigationFailure, toLocation, from)
+      )
+      .then((failure: NavigationFailure | NavigationRedirectError | void) => {
+        if (failure) {
+          if (
+            isNavigationFailure(failure, ErrorTypes.NAVIGATION_GUARD_REDIRECT)
+          ) {
+            if (
+              __DEV__ &&
+              // we are redirecting to the same location we were already at
+              isSameRouteLocation(
+                stringifyQuery,
+                resolve(failure.to),
+                toLocation
+              ) &&
+              // and we have done it a couple of times
+              redirectedFrom &&
+              // @ts-expect-error: added only in dev
+              (redirectedFrom._count = redirectedFrom._count
+                ? // @ts-expect-error
+                  redirectedFrom._count + 1
+                : 1) > 10
+            ) {
+              warn(
+                `Detected an infinite redirection in a navigation guard when going from "${from.fullPath}" to "${toLocation.fullPath}". Aborting to avoid a Stack Overflow. This will break in production if not fixed.`
+              )
+              return Promise.reject(
+                new Error('Infinite redirect in navigation guard')
+              )
             }
+
+            // FIXME: find a way to keep the return pattern of promises to handle reusing the promise maybe by
+            // passing the resolve, reject as parameters to pushWithRedirect
+
+            return pushWithRedirect(
+              // keep options
+              assign(locationAsObject(failure.to), {
+                state: data,
+                force,
+                replace,
+              }),
+              // preserve the original redirectedFrom if any
+              redirectedFrom || toLocation
+            )
+          }
+          // we had a failure so we cannot change the URL
+        } else {
+          // Nothing prevented the navigation to happen, we can update the URL
+          pendingNavigation.value = null
+          failure = finalizeNavigation(
+            toLocation as RouteLocationNormalizedLoaded,
+            from,
+            true,
+            replace,
+            data
           )
-      }
-    ))
+        }
+        // we updated the URL and currentRoute, this will update the rendered router view
+        // we move into the second phase of a navigation: awaiting suspended routes
+        return new Promise((promiseResolve, promiseReject) => {
+          // FIXME: if we reject here we need to restore the navigation like we do in the setupListeners
+          // this would be so much easier with the App History API
+          rejectPendingNavigation = promiseReject
+          resolvePendingNavigation = () => {
+            triggerAfterEach(
+              toLocation as RouteLocationNormalizedLoaded,
+              from,
+              failure as any
+            )
+            promiseResolve(failure as any)
+          }
+          valueToResolveOrError = failure
+        })
+      }))
   }
 
   /**