From: Eduardo San Martin Morote Date: Tue, 8 Feb 2022 23:19:46 +0000 (+0100) Subject: feat: wip wait for suspense X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2cdb08a2c5f09a1a18536269b1db27204e796981;p=thirdparty%2Fvuejs%2Frouter.git feat: wip wait for suspense --- diff --git a/e2e/suspense-view/index.html b/e2e/suspense-view/index.html new file mode 100644 index 00000000..16d284b7 --- /dev/null +++ b/e2e/suspense-view/index.html @@ -0,0 +1,19 @@ + + + + + + + Vue Router e2e tests - Suspense View + + + + + + << Back to Homepage +
+ +
+ + + diff --git a/e2e/suspense-view/index.ts b/e2e/suspense-view/index.ts new file mode 100644 index 00000000..c9e88792 --- /dev/null +++ b/e2e/suspense-view/index.ts @@ -0,0 +1,208 @@ +import '../global.css' +import { + createRouter, + createWebHistory, + onBeforeRouteUpdate, + onBeforeRouteLeave, + useRoute, + SusRouterView, +} from '../../src' +import { + createApp, + ref, + reactive, + defineComponent, + FunctionalComponent, + h, + onErrorCaptured, + defineAsyncComponent, +} from 'vue' + +const Home = defineComponent({ + name: 'Home', + template: ` +
+

Home

+
+ `, +}) + +const delay = (t: number) => new Promise(r => setTimeout(r, t)) + +const AsyncImport = defineAsyncComponent(async () => { + await delay(1000) + console.log('finished loading async component') + return defineComponent({ + name: 'AsyncImport', + beforeMount() { + console.log('done') + }, + template: `
AsyncImport
`, + }) +}) + +const n = ref(0) + +setInterval(() => { + n.value++ +}, 1000) + +/** + * creates a component that logs the guards + * @param name + */ +function createAsyncComponent(key: string, isAsync = true) { + return defineComponent({ + name: key, + components: { AsyncImport }, + template: `
${key}: n = {{n}}.
`, + + setup() { + const route = useRoute() + const shouldFail = !!route.query.fail + + console.log(`Setup of ${key}...`) + + const ret = { n } + + return isAsync + ? delay(2000).then(() => { + console.log(`finished setup of ${key}`) + + return shouldFail ? Promise.reject(new Error('failed')) : ret + }) + : ret + }, + }) +} + +function createAsyncNestedComponent(key: string) { + return defineComponent({ + name: key, + template: `
${key}: + + + + +
`, + + setup() { + const route = useRoute() + const shouldFail = !!route.query.fail + + console.log(`Setup of ${key}...`) + + return delay(100).then(() => + shouldFail ? Promise.reject(new Error('failed')) : {} + ) + }, + }) +} + +const Foo = createAsyncComponent('Foo', false) +const FooAsync = createAsyncComponent('FooAsync') +const PassThroughView: FunctionalComponent = () => h(SusRouterView) +PassThroughView.displayName = 'SusRouterView' + +const webHistory = createWebHistory('/suspense-view') +const router = createRouter({ + history: webHistory, + routes: [ + { path: '/', component: Home }, + { + path: '/foo', + component: Foo, + }, + { + path: '/foo-async', + component: FooAsync, + }, + { + path: '/nested', + component: PassThroughView, + children: [ + { path: 'foo', component: Foo }, + { path: 'foo-async', component: FooAsync }, + ], + }, + { + path: '/nested-async', + component: createAsyncNestedComponent('NestedAsync'), + children: [ + { path: 'foo', component: Foo }, + { path: 'foo-async', component: FooAsync }, + ], + }, + ], +}) +const shouldFail = ref(false) +const app = createApp({ + template: ` +

Suspense

+ + 
+route: {{ $route.fullPath }}
+
+ + + + + + + + + + + + `, + setup() { + onErrorCaptured(err => { + console.log('❌ From Suspense', err) + }) + return { clear: console.clear, shouldFail } + }, +}) +app.component('SusRouterView', SusRouterView) +app.config.globalProperties.log = console.log + +router.beforeEach((to, from) => { + console.log('-'.repeat(10)) + console.log(`🏎 ${from.fullPath} -> ${to.fullPath}`) + 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}`) + } else { + console.log(`🏁 ${from.fullPath} -> ${to.fullPath}`) + } +}) +router.onError((error, to, from) => { + console.log(`💥 ${from.fullPath} -> ${to.fullPath}`) + console.error(error) + console.log('-'.repeat(10)) +}) +app.use(router) + +app.mount('#app') + +window.r = router diff --git a/e2e/suspense/index.ts b/e2e/suspense/index.ts index fe1bc179..b0f5d58f 100644 --- a/e2e/suspense/index.ts +++ b/e2e/suspense/index.ts @@ -97,7 +97,7 @@ const PassThroughViewSuspense: FunctionalComponent = (_, { emit }) => PassThroughViewSuspense.displayName = 'PTVS' PassThroughViewSuspense.emits = ['pending', 'resolve'] -const webHistory = createWebHistory('/' + __dirname) +const webHistory = createWebHistory('/suspense') const router = createRouter({ history: webHistory, routes: [ diff --git a/e2e/suspense/notes.md b/e2e/suspense/notes.md index 41f0a611..2039c4c8 100644 --- a/e2e/suspense/notes.md +++ b/e2e/suspense/notes.md @@ -122,27 +122,26 @@ This is an idea of integrating better with Suspense and having one single naviga - 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 +- It shouldn't change the existing behavior when unused + +- **Should it also trigger when leaving?** I think it makes more sense for it to trigger only on entering or updating (cf the example below) ### API usage -```js +```vue + ``` Let's consider these routes: @@ -153,14 +152,14 @@ Let's consider these routes: 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 + - Calls `getUser(1)` thanks to `onBeforeNavigation()` + - Keeps Home (`/`) visible until it resolves or fails + - resolves: finish navigation (triggers `afterEach()`), 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)` + - Also calls `getUser(2)` thanks to `onBeforeNavigation()` - Keeps User 1 (`/users/1`) visible until resolves or fails - - resolves: switch to `/users/2` and display the view with the content ready + - resolves: (same as above) 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()` @@ -179,6 +178,12 @@ This would be the expected behavior: The implementation for this hook requires displaying multiple router views at the same time: the pending view we are navigating to and the current +- To avoid +- We need to wrap every component with Suspense (even nested ones) +- Multiple Suspenses can resolve but we need to wait for all of them to resolve + - `onBeforeNavigation()` could increment a counter + - Without it we can only support it in view components: we count `to.matched.length` + ## Other notes - RouterView could expose the `depth` (number) alongside `Component` and `route`. It is used to get the matched view from `route.matched[depth]` diff --git a/src/SusRouterView.ts b/src/SusRouterView.ts new file mode 100644 index 00000000..42e87530 --- /dev/null +++ b/src/SusRouterView.ts @@ -0,0 +1,258 @@ +import { + h, + inject, + provide, + defineComponent, + PropType, + ref, + ComponentPublicInstance, + VNodeProps, + getCurrentInstance, + computed, + AllowedComponentProps, + ComponentCustomProps, + watch, + Slot, + VNode, + Suspense, +} from 'vue' +import { + RouteLocationNormalized, + RouteLocationNormalizedLoaded, + RouteLocationMatched, +} from './types' +import { + matchedRouteKey, + viewDepthKey, + routerViewLocationKey, + pendingViewKey, +} from './injectionSymbols' +import { assign, isBrowser } from './utils' +import { warn } from './warning' +import { isSameRouteRecord } from './location' + +export interface SusRouterViewProps { + name?: string + // allow looser type for user facing api + route?: RouteLocationNormalized +} + +export interface RouterViewDevtoolsContext + extends Pick { + depth: number +} + +export const SusRouterViewImpl = /*#__PURE__*/ defineComponent({ + name: 'SusRouterView', + // #674 we manually inherit them + inheritAttrs: false, + props: { + name: { + type: String as PropType, + default: 'default', + }, + route: Object as PropType, + }, + emits: ['resolve', 'pending'], + + setup(props, { attrs, slots, emit }) { + __DEV__ && warnDeprecatedUsage() + + const injectedRoute = inject(routerViewLocationKey)! + const routeToDisplay = computed(() => props.route || injectedRoute.value) + const depth = inject(viewDepthKey, 0) + const matchedRouteRef = computed( + () => routeToDisplay.value.matched[depth] + ) + + provide(viewDepthKey, depth + 1) + provide(matchedRouteKey, matchedRouteRef) + provide(routerViewLocationKey, routeToDisplay) + + const viewRef = ref() + + // watch at the same time the component instance, the route record we are + // rendering, and the name + watch( + () => [viewRef.value, matchedRouteRef.value, props.name] as const, + ([instance, to, name], [oldInstance, from, oldName]) => { + // copy reused instances + if (to) { + // this will update the instance for new instances as well as reused + // instances when navigating to a new route + to.instances[name] = instance + // the component instance is reused for a different route or name so + // we copy any saved update or leave guards. With async setup, the + // mounting component will mount before the matchedRoute changes, + // making instance === oldInstance, so we check if guards have been + // added before. This works because we remove guards when + // unmounting/deactivating components + if (from && from !== to && instance && instance === oldInstance) { + if (!to.leaveGuards.size) { + to.leaveGuards = from.leaveGuards + } + if (!to.updateGuards.size) { + to.updateGuards = from.updateGuards + } + } + } + + // trigger beforeRouteEnter next callbacks + if ( + instance && + to && + // if there is no instance but to and from are the same this might be + // the first visit + (!from || !isSameRouteRecord(to, from) || !oldInstance) + ) { + ;(to.enterCallbacks[name] || []).forEach(callback => + callback(instance) + ) + } + }, + { flush: 'post' } + ) + + const addPendingView = inject(pendingViewKey)! + + return () => { + const route = routeToDisplay.value + const matchedRoute = matchedRouteRef.value + const ViewComponent = matchedRoute && matchedRoute.components[props.name] + // we need the value at the time we render because when we unmount, we + // navigated to a different location so the value is different + const currentName = props.name + + if (!ViewComponent) { + return normalizeSlot(slots.default, { Component: ViewComponent, route }) + } + + // props from route configuration + const routePropsOption = matchedRoute!.props[props.name] + const routeProps = routePropsOption + ? routePropsOption === true + ? route.params + : typeof routePropsOption === 'function' + ? routePropsOption(route) + : routePropsOption + : null + + const onVnodeUnmounted: VNodeProps['onVnodeUnmounted'] = vnode => { + // remove the instance reference to prevent leak + if (vnode.component!.isUnmounted) { + matchedRoute!.instances[currentName] = null + } + } + + // FIXME: only because Suspense doesn't emit the initial pending + // emit('pending') + + let unregisterPendingView: ReturnType + + const component = h( + Suspense, + { + onPending: () => { + unregisterPendingView = addPendingView(Symbol()) + emit('pending', String(ViewComponent.name || 'unnamed')) + }, + onResolve: () => { + unregisterPendingView && unregisterPendingView() + emit('resolve', String(ViewComponent.name || 'unnamed')) + }, + // onResolve, + }, + { + fallback: slots.fallback, + default: () => + h( + ViewComponent, + assign({}, routeProps, attrs, { + onVnodeUnmounted, + ref: viewRef, + }) + ), + } + ) + + if ( + (__DEV__ || __FEATURE_PROD_DEVTOOLS__) && + isBrowser && + component.ref + ) { + // TODO: can display if it's an alias, its props + const info: RouterViewDevtoolsContext = { + depth, + name: matchedRoute.name, + path: matchedRoute.path, + meta: matchedRoute.meta, + } + + const internalInstances = Array.isArray(component.ref) + ? component.ref.map(r => r.i) + : [component.ref.i] + + internalInstances.forEach(instance => { + // @ts-expect-error + instance.__vrv_devtools = info + }) + } + + return ( + // pass the vnode to the slot as a prop. + // h and both accept vnodes + normalizeSlot(slots.default, { Component: component, route }) || + component + ) + } + }, +}) + +function normalizeSlot(slot: Slot | undefined, data: any) { + if (!slot) return null + const slotContent = slot(data) + return slotContent.length === 1 ? slotContent[0] : slotContent +} + +// export the public type for h/tsx inference +// also to avoid inline import() in generated d.ts files +/** + * Component to display the current route the user is at. + */ +export const SusRouterView = SusRouterViewImpl as unknown as { + new (): { + $props: AllowedComponentProps & + ComponentCustomProps & + VNodeProps & + SusRouterViewProps + + $slots: { + default: (arg: { + Component: VNode + route: RouteLocationNormalizedLoaded + }) => VNode[] + } + } +} + +// warn against deprecated usage with & +// due to functional component being no longer eager in Vue 3 +function warnDeprecatedUsage() { + const instance = getCurrentInstance()! + const parentName = instance.parent && instance.parent.type.name + if ( + parentName && + (parentName === 'KeepAlive' || parentName.includes('Transition')) + ) { + const comp = parentName === 'KeepAlive' ? 'keep-alive' : 'transition' + warn( + ` can no longer be used directly inside or .\n` + + `Use slot props instead:\n\n` + + `\n` + + ` <${comp}>\n` + + ` \n` + + ` \n` + + `` + ) + } +} diff --git a/src/index.ts b/src/index.ts index adfd8207..ce85d403 100644 --- a/src/index.ts +++ b/src/index.ts @@ -71,6 +71,8 @@ export { RouterLink, useLink } from './RouterLink' export type { RouterLinkProps, UseLinkOptions } from './RouterLink' export { RouterView } from './RouterView' export type { RouterViewProps } from './RouterView' +export { SusRouterView } from './SusRouterView' +export type { SusRouterViewProps } from './SusRouterView' export * from './useApi' diff --git a/src/injectionSymbols.ts b/src/injectionSymbols.ts index 0d345ea1..633cd337 100644 --- a/src/injectionSymbols.ts +++ b/src/injectionSymbols.ts @@ -63,3 +63,7 @@ export const routeLocationKey = /*#__PURE__*/ PolySymbol( export const routerViewLocationKey = /*#__PURE__*/ PolySymbol( __DEV__ ? 'router view location' : 'rvl' ) as InjectionKey> + +export const pendingViewKey = /*#__PURE__*/ PolySymbol( + __DEV__ ? 'pending view' : 'pv' +) as InjectionKey<(view: any) => () => void> diff --git a/src/router.ts b/src/router.ts index da3170c5..16dfd3d0 100644 --- a/src/router.ts +++ b/src/router.ts @@ -50,6 +50,7 @@ import { reactive, unref, computed, + ShallowRef, } from 'vue' import { RouteRecord, RouteRecordNormalized } from './matcher/types' import { @@ -63,6 +64,7 @@ import { warn } from './warning' import { RouterLink } from './RouterLink' import { RouterView } from './RouterView' import { + pendingViewKey, routeLocationKey, routerKey, routerViewLocationKey, @@ -191,7 +193,12 @@ export interface Router { /** * Current {@link RouteLocationNormalized} */ - readonly currentRoute: Ref + readonly currentRoute: ShallowRef + + readonly pendingNavigation: ShallowRef< + null | undefined | Promise + > + /** * Original options object passed to create the Router */ @@ -370,6 +377,24 @@ export function createRouter(options: RouterOptions): Router { START_LOCATION_NORMALIZED ) let pendingLocation: RouteLocation = START_LOCATION_NORMALIZED + const pendingViews = new Set() + const pendingNavigation = shallowRef< + undefined | null | ReturnType + >() + let valueToResolveOrError: any + let resolvePendingNavigation: (resolvedValue: any) => void = noop + let rejectPendingNavigation: (error: any) => void = noop + + function addPendingView(view: any) { + pendingViews.add(view) + + return () => { + pendingViews.delete(view) + if (!pendingViews.size) { + resolvePendingNavigation(valueToResolveOrError) + } + } + } // leave the scrollRestoration if no scrollBehavior is provided if (isBrowser && options.scrollBehavior && 'scrollRestoration' in history) { @@ -679,70 +704,90 @@ export function createRouter(options: RouterOptions): Router { ) } - 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') - ) - } + pendingViews.clear() - 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 - failure = finalizeNavigation( - toLocation as RouteLocationNormalizedLoaded, - from, - true, - replace, - data + 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) ) - } - triggerAfterEach( - toLocation as RouteLocationNormalizedLoaded, - from, - failure - ) - return failure - }) + .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 + } + ) + } + )) } /** @@ -888,6 +933,11 @@ export function createRouter(options: RouterOptions): Router { // navigation is confirmed, call afterGuards // TODO: wrap with error handlers for (const guard of afterGuards.list()) guard(to, from, failure) + + // TODO: moving this here is technically a breaking change maybe as it would mean the afterEach trigger before any + // afterEach but I think it's rather a fix. + // FIXME: this breaks a lot of tests + markAsReady() } /** @@ -931,8 +981,6 @@ export function createRouter(options: RouterOptions): Router { // accept current navigation currentRoute.value = toLocation handleScroll(toLocation, from, isPush, isFirstNavigation) - - markAsReady() } let removeHistoryListener: () => void | undefined @@ -1140,6 +1188,7 @@ export function createRouter(options: RouterOptions): Router { const router: Router = { currentRoute, + pendingNavigation, addRoute, removeRoute, @@ -1202,6 +1251,7 @@ export function createRouter(options: RouterOptions): Router { app.provide(routerKey, router) app.provide(routeLocationKey, reactive(reactiveRoute)) app.provide(routerViewLocationKey, currentRoute) + app.provide(pendingViewKey, addPendingView) const unmountApp = app.unmount installedApps.add(app)