From: Eduardo San Martin Morote Date: Wed, 30 Jun 2021 12:44:10 +0000 (+0200) Subject: chore: stuff X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=07d6f4b3ebba616ff8640ccd41c5987ec47be1fe;p=thirdparty%2Fvuejs%2Frouter.git chore: stuff --- diff --git a/e2e/suspense/index.ts b/e2e/suspense/index.ts index 04cd2a05..fe1bc179 100644 --- a/e2e/suspense/index.ts +++ b/e2e/suspense/index.ts @@ -14,6 +14,8 @@ import { defineComponent, FunctionalComponent, h, + Suspense, + onErrorCaptured, } from 'vue' const Home = defineComponent({ @@ -35,7 +37,9 @@ const state = reactive({ const delay = (t: number) => new Promise(r => setTimeout(r, t)) /** - * creates a component that logs the guards + * creates a component that logs the guards and that can also return a promise + * in `setup()` + * * @param name */ function createTestComponent(key: string, isAsync = false) { @@ -57,7 +61,7 @@ function createTestComponent(key: string, isAsync = false) { const shouldFail = !!route.query.fail return isAsync - ? delay(100).then(() => + ? delay(1000).then(() => shouldFail ? Promise.reject(new Error('failed')) : {} ) : {} @@ -67,10 +71,33 @@ function createTestComponent(key: string, isAsync = false) { const Foo = createTestComponent('Foo') const FooAsync = createTestComponent('FooAsync', true) + +// A Pass Through view that just renders a router view to allow nested routes to +// just reuse the path without using the layout system it can provide const PassThroughView: FunctionalComponent = () => h(RouterView) PassThroughView.displayName = 'RouterView' -const webHistory = createWebHistory('/suspense') +const PassThroughViewSuspense: FunctionalComponent = (_, { emit }) => + h(RouterView, null, { + default: ({ Component }: any) => + h( + Suspense, + { + onPending: () => emit('pending'), + onResolve: () => emit('resolve'), + timeout: 500, + }, + { + default: () => h(Component), + fallback: () => h('p', 'Loading nested...'), + } + ), + }) + +PassThroughViewSuspense.displayName = 'PTVS' +PassThroughViewSuspense.emits = ['pending', 'resolve'] + +const webHistory = createWebHistory('/' + __dirname) const router = createRouter({ history: webHistory, routes: [ @@ -91,9 +118,19 @@ const router = createRouter({ { path: 'two', component: createTestComponent('two', true) }, ], }, + { + path: '/nested-suspense', + component: PassThroughViewSuspense, + children: [ + { path: 'one', component: createTestComponent('one', true) }, + { path: 'two', component: createTestComponent('two', true) }, + ], + }, ], }) + const shouldFail = ref(false) + const app = createApp({ template: `

Suspense

@@ -107,7 +144,9 @@ leaves: {{ state.leave }} - 
{{ logs.join('\\n') }}
+
+ + 
{{ logs.join('\\n') }}
@@ -118,20 +157,50 @@ leaves: {{ state.leave }}
  • {{ route.fullPath }}
  • /nested/one
  • /nested/two
    • +
  • /nested-suspense/one
    • +
  • /nested-suspense/two
    • - - + + + `, setup() { - return { state, logs, log: console.log, shouldFail } + onErrorCaptured(error => { + console.warn('Error captured', error) + rejectPending(error) + return false + }) + + function resolvePending() { + console.log('resolve') + pendingContext?.resolve() + } + + function rejectPending(err: any) { + pendingContext?.reject(err) + } + + return { + state, + logs, + log: console.log, + resolvePending, + shouldFail, + // TODO: remove after showing tests + TODO: true, + createPendingContext, + } }, }) router.beforeEach(to => { + console.log('beforeEach') if (shouldFail.value && !to.query.fail) return { ...to, query: { ...to.query, fail: 'yes' } } return @@ -141,3 +210,37 @@ app.use(router) window.r = router app.mount('#app') + +// code to handle the pending context on suspense + +router.beforeEach(async () => { + // const pending = createPendingContext() + // await pending.promise +}) + +interface PendingContext { + resolve(): void + reject(error?: any): void + + promise: Promise +} + +let pendingContext: PendingContext | null = null + +function createPendingContext(): PendingContext { + // reject any ongoing pending navigation + if (pendingContext) { + pendingContext.reject(new Error('New Navigation')) + } + + let resolve: PendingContext['resolve'] + let reject: PendingContext['reject'] + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + + pendingContext = { promise, resolve: resolve!, reject: reject! } + + return pendingContext +} diff --git a/e2e/suspense/notes.md b/e2e/suspense/notes.md new file mode 100644 index 00000000..41f0a611 --- /dev/null +++ b/e2e/suspense/notes.md @@ -0,0 +1,184 @@ +# Suspense + Router View + +- Ignore the log of navigation guards + +When toggling between two routes (or components), if one is async and we are using Suspense around it, Suspense will display the current component until the pending one resolves. **This is the desired behavior**. Can we make it work the same for nested async components? + +This is the [current code](https://github.com/nuxt/framework/blob/main/packages/pages/src/runtime/page.vue) for ``: + +```vue + + + +``` + +Right now it pretty much replaces `` in Nuxt apps, so it's used for nested pages as well + +## Async Views not linked to navigation + +Right now, the router will navigate to a location and only then, `async setup` will run. This can still be fine as users do not necessarily need to put fetching logic inside navigation (Although I think it makes more sense unless you are handling the loading state manually). + +### Advantages + +- Simplest code as it only requires changes in the `template`: + ```html + + + + + + ``` + `pendingHandler` and `resolveHandler` are completely optional +- Displays the previous view while the new one is pending + +### Problems / Solutions + +- Can't handle Nested Routes [#Problem 1](#problem-1) +- Route updates eagerly, which can be confusing and is different from fetching during navigation + - This cannot be solved without [a proper integration with vue router](#onBeforeNavigation) +- Errors (technically Promise rejections) still try displaying the async component that failed + - **Possible solutions**: + - An `error` slot to let the user show something + - Show (or rather stay with) the previous content if no `error` slot is provided because it's very likely to fail anyway and error something harder to debug because of initialized/inexistent properties + - An `error` event + +### Questions + +- Is the `fallback` event really useful? Isn't `pending` enough? What is the point of knowing when `fallback` is displayed. +- Could we have a new nested pending event that lets us know when any nested suspense goes pending? Same for nested resolve and maybe error. These events would have to be emitted once if multiple nested suspense go pending at the same tick. + +--- + +Right now the Router doesn't listen for any events coming from suspense. The e2e test in this folder is meant to listen and play around with a possible integration of Vue Router with Suspense. + +## Use case + +This is a more realistic scenario of data fetching in `async setup()`: + +```js +export default { + async setup() { + const route = useRoute() + // any error could be caught by navigation guards if we listen to `onErrorCaptured() + const user = ref(await getUser(route.params.id)) + + return { user } + }, +} +``` + +## Problem 1 + +Run the example with `yarn run dev:e2e` and go to [the suspense test page](http://localhost:8080/suspense). It has a few views that aren't async, nested views **without nested Suspense** and nested views **with nested suspense**. + +All async operations take 1s to resolve. They can also reject by adding `?fail=yes` to the URL or by checking the only checkbox on the page. +The example is also displaying a fallback slot with a 500ms timeout to see it midway through the pending phase. + +Right now, these behaviors are undesired: + +- Going from `/nested/one` to `/nested/two` displays a blank view while nested two loads. It should display one and then _loading root_ instead while pending +- Going from `/nested/one` to `/async-foo` displays a blank view while async foo loads. It should display one instead before displaying _loading root_ while pending +- Going from `/nested-suspense/one` to `/foo-async` displays a blank view while foo async loads. It should display one before displaying _loading root_. It also creates a weird error `Invalid vnode type when creating vnode`: + ``` + runtime-core.esm-bundler.js:38 [Vue warn]: Invalid vnode type when creating vnode: undefined. + at + at ref=Ref<

    ​Loading nested...​

    ​ > class="view" > + at + at + ``` +- Going from `/foo-async` to `/nested-suspense/one` displays _loading nested_ right away instead of displaying foo async for 500ms (as if we were initially visiting) +- Going from `/nested-suspense/one` to `/nested/two` breaks the application by leaving a loading node there. + +Ideas: + +- Would it be possible to display the whole current tree when a child goes _pending_? We could still display their nested fallback slots if they have any +- Is this possible without using nested Suspense? I think it's fine to require users to use nested Suspense with nested views. It could be abstracted anyway (e.g. with NuxtPage). + +## onBeforeNavigation + +> ⚠️ : this is a different problem from the one above + +This is an idea of integrating better with Suspense and having one single navigation guard that triggers on enter and update. It goes further and the idea is to move to more intuitive APIs and keep improving vue router. Right now, we can achieve something similar with a `router.beforeResolve()` hook and saving data in `to.meta` but it disconnects the asynchronous operation from the view component and therefore is limited and not intuitive. + +### Needs + +- Become part of navigation: the URL should not change until all `` resolve +- Allows the user to display a `fallback` slot and use the `timeout` prop to control when it appears. Note there could be a new RouterView Component that accept those slots and forward them to `Suspense`. +- Abort the navigation when async setup errors and trigger `router.onError()` but still display the current route + +### API usage + +```js +import { onBeforeNavigation } from 'vue-router' +import { getUser } from './api' + +/** + * This is the component for /users/:id, it fetches the user information and display it. + */ +export default { + async setup() { + const user = ref() + + await onBeforeNavigation(async (to, from) => { + user.value = await getUser(to.params.id) + }) + + return { user } + }, +} +``` + +Let's consider these routes: + +- Home `/`: Not Async +- User `/users/:id`: Async, should fetch the user data to display it + +This would be the expected behavior: + +- Going from `/` to `/users/1` (Entering): + - Calls `getUser(1)` + - Keeps Home (`/`) visible until resolves or fails + - resolves: switch to `/users/1` and display the view with the content ready + - fails: triggers `router.onError()`, stays at Home +- Going from `/users/1` to `/users/2` (Updating): + - Calls `getUser(2)` + - Keeps User 1 (`/users/1`) visible until resolves or fails + - resolves: switch to `/users/2` and display the view with the content ready + - fails: triggers `router.onError()`, stays at User 1 +- Going from `/users/2` to `/` (Leaving): + - Directly goes to Home without calling `getUser()` + +## Pros + +- Fully integrates with the router navigation + - Allows global loaders (progress bar) to be attached to `beforeEach()`, `afterEach()`, and `onError()` + +## Cons + +- Complex implementation +- Could be fragile and break easily too (?) + +## Implementation + +The implementation for this hook requires displaying multiple router views at the same time: the pending view we are navigating to and the current + +## Other notes + +- RouterView could expose the `depth` (number) alongside `Component` and `route`. It is used to get the matched view from `route.matched[depth]`