From c518750902e11fc07add2535833ef68eeff33885 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Thu, 27 Aug 2020 14:31:41 +0200 Subject: [PATCH] refactor(router-view): use a key instead of watcher --- e2e/guards-instances/index.ts | 21 ++++++++++-- e2e/specs/guards-instances.js | 53 +++++++++++++++++++++---------- src/RouterView.ts | 60 +++++++++++++++++++---------------- 3 files changed, 86 insertions(+), 48 deletions(-) diff --git a/e2e/guards-instances/index.ts b/e2e/guards-instances/index.ts index 0a82dfd66..1ad414a22 100644 --- a/e2e/guards-instances/index.ts +++ b/e2e/guards-instances/index.ts @@ -1,4 +1,9 @@ -import { createRouter, createWebHistory } from '../../src' +import { + createRouter, + createWebHistory, + onBeforeRouteUpdate, + onBeforeRouteLeave, +} from '../../src' import { createApp, ref, reactive, defineComponent } from 'vue' // override existing style on dev with shorter times @@ -58,6 +63,16 @@ const Foo = defineComponent({ state.leave++ logs.value.push(`leave ${from.path} - ${to.path}`) }, + + setup() { + onBeforeRouteUpdate((to, from) => { + console.log(`setup update ${from.path} - ${to.path}`) + }) + onBeforeRouteLeave((to, from) => { + console.log(`setup leave ${from.path} - ${to.path}`) + }) + return {} + }, }) const webHistory = createWebHistory('/' + __dirname) @@ -121,9 +136,9 @@ leaves: {{ state.leave }} diff --git a/e2e/specs/guards-instances.js b/e2e/specs/guards-instances.js index 50efe5a43..eea553198 100644 --- a/e2e/specs/guards-instances.js +++ b/e2e/specs/guards-instances.js @@ -5,13 +5,13 @@ function testCase(browser) { .click('li:nth-child(2) a') .assert.containsText('.view', 'foo 1') .click('li:nth-child(3) a') - .assert.containsText('.view', 'foo 2') + .assert.containsText('.view', 'foo 1') .click('li:nth-child(4) a') - .assert.containsText('.view', 'foo 2') + .assert.containsText('.view', 'foo 1') .click('li:nth-child(5) a') - .assert.containsText('.view', 'foo 2') + .assert.containsText('.view', 'foo 1') .click('li:nth-child(2) a') - .assert.containsText('.view', 'foo 3') + .assert.containsText('.view', 'foo') .assert.containsText( '#logs', [ @@ -64,23 +64,28 @@ module.exports = { .click('li:nth-child(1) a') // keep alive keeps the correct instance .click('li:nth-child(2) a') + .assert.containsText('.view', 'foo 3') + .click('li:nth-child(1) a') + .click('li:nth-child(2) a') .assert.containsText('.view', 'foo 4') + .click('li:nth-child(3) a') + .assert.containsText('.view', 'foo 2') browser.end() }, - // /** @type {import('nightwatch').NightwatchTest} */ - // 'guards instances transition': function (browser) { - // browser - // .url('http://localhost:8080/guards-instances/') - // .waitForElementPresent('#app > *', 1000) + /** @type {import('nightwatch').NightwatchTest} */ + 'guards instances transition': function (browser) { + browser + .url('http://localhost:8080/guards-instances/') + .waitForElementPresent('#app > *', 1000) - // .click('#test-transition') + .click('#test-transition') - // testCase(browser) + testCase(browser) - // browser.end() - // }, + browser.end() + }, /** @type {import('nightwatch').NightwatchTest} */ 'guards instances keyed': function (browser) { @@ -97,6 +102,13 @@ module.exports = { // the query is used as a key resetting the enter count .click('li:nth-child(6) a') .assert.containsText('.view', 'foo 0') + // changing both the route and mounting the component + .click('li:nth-child(2) a') + .assert.containsText('.view', 'foo 1') + .click('li:nth-child(6) a') + .assert.containsText('.view', 'foo 1') + .click('li:nth-child(2) a') + .assert.containsText('.view', 'foo 1') browser.end() }, @@ -112,18 +124,25 @@ module.exports = { testCase(browser) browser + .click('li:nth-child(1) a') + // keep alive keeps the correct instance + .click('li:nth-child(2) a') + .assert.containsText('.view', 'foo 3') + .click('li:nth-child(1) a') + .click('li:nth-child(2) a') + .assert.containsText('.view', 'foo 4') + .click('li:nth-child(3) a') + .assert.containsText('.view', 'foo 2') + .click('li:nth-child(5) a') // the query is used as a key resetting the enter count .click('li:nth-child(6) a') .assert.containsText('.view', 'foo 0') - // going back to /foo - .click('li:nth-child(2) a') - .assert.containsText('.view', 'foo 5') .click('li:nth-child(1) a') .click('li:nth-child(6) a') .assert.containsText('.view', 'foo 1') .click('li:nth-child(5) a') - .assert.containsText('.view', 'foo 5') + .assert.containsText('.view', 'foo 2') browser.end() }, diff --git a/src/RouterView.ts b/src/RouterView.ts index f5cc45f3d..480d05739 100644 --- a/src/RouterView.ts +++ b/src/RouterView.ts @@ -11,9 +11,12 @@ import { computed, AllowedComponentProps, ComponentCustomProps, - watch, } from 'vue' -import { RouteLocationNormalized, RouteLocationNormalizedLoaded } from './types' +import { + RouteLocationNormalized, + RouteLocationNormalizedLoaded, + RouteLocationMatched, +} from './types' import { matchedRouteKey, viewDepthKey, @@ -43,7 +46,7 @@ export const RouterViewImpl = defineComponent({ const injectedRoute = inject(routeLocationKey)! const depth = inject(viewDepthKey, 0) - const matchedRouteRef = computed( + const matchedRouteRef = computed( () => (props.route || injectedRoute).matched[depth] ) @@ -55,35 +58,39 @@ export const RouterViewImpl = defineComponent({ // when the same component is used in different routes, the onVnodeMounted // hook doesn't trigger, so we need to observe the changing route to update // the instance on the record - watch(matchedRouteRef, to => { - const currentName = props.name - // to can be null if there isn't a matched route, e.g. not found - if (to && !to.instances[currentName]) { - to.instances[currentName] = viewRef.value - // trigger enter callbacks when different routes only - if (viewRef.value) { - ;(to.enterCallbacks[currentName] || []).forEach(callback => - callback(viewRef.value!) - ) - // avoid double calls since watch is called before the onVnodeMounted - to.enterCallbacks[currentName] = [] - } - } - }) + // watch(matchedRouteRef, to => { + // const currentName = props.name + // // to can be null if there isn't a matched route, e.g. not found + // if (to && !to.instances[currentName]) { + // to.instances[currentName] = viewRef.value + // // trigger enter callbacks when different routes only + // if (viewRef.value) { + // ;(to.enterCallbacks[currentName] || []).forEach(callback => + // callback(viewRef.value!) + // ) + // // avoid double calls since watch is called before the onVnodeMounted + // to.enterCallbacks[currentName] = [] + // } + // } + // }) return () => { const route = props.route || injectedRoute 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 + const key = matchedRoute && currentName + matchedRoute.path if (!ViewComponent) { return slots.default - ? slots.default({ Component: ViewComponent, route }) + ? slots.default({ Component: ViewComponent, route, key }) : null } // props from route configuration - const routePropsOption = matchedRoute.props[props.name] + const routePropsOption = matchedRoute!.props[props.name] const routeProps = routePropsOption ? routePropsOption === true ? route.params @@ -92,23 +99,20 @@ export const RouterViewImpl = defineComponent({ : routePropsOption : null - // 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 const onVnodeMounted = () => { - matchedRoute.instances[currentName] = viewRef.value - ;(matchedRoute.enterCallbacks[currentName] || []).forEach(callback => + matchedRoute!.instances[currentName] = viewRef.value + ;(matchedRoute!.enterCallbacks[currentName] || []).forEach(callback => callback(viewRef.value!) ) } const onVnodeUnmounted = () => { // remove the instance reference to prevent leak - matchedRoute.instances[currentName] = null + matchedRoute!.instances[currentName] = null } const component = h( ViewComponent, - assign({}, routeProps, attrs, { + assign({ key }, routeProps, attrs, { onVnodeMounted, onVnodeUnmounted, ref: viewRef, @@ -119,7 +123,7 @@ export const RouterViewImpl = defineComponent({ // pass the vnode to the slot as a prop. // h and both accept vnodes slots.default - ? slots.default({ Component: component, route }) + ? slots.default({ Component: component, route, key }) : component ) }