Skip to content

Commit

Permalink
refactor(router-view): use a key instead of watcher
Browse files Browse the repository at this point in the history
  • Loading branch information
@posva
posva committed Aug 27, 2020
1 parent aba6c44 commit c518750
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 48 deletions.
21 changes: 18 additions & 3 deletions e2e/guards-instances/index.ts
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -121,9 +136,9 @@ leaves: {{ state.leave }}
<router-view :key="$route.query.foo" class="view" />
</template>
<template v-else-if="testCase === 'keepalivekeyed'">
<router-view v-slot="{ Component }" >
<router-view v-slot="{ Component, key }" >
<keep-alive>
<component :is="Component" :key="$route.query.foo" class="view" />
<component :is="Component" :key="$route.query.foo || key" class="view" />
</keep-alive>
</router-view>
</template>
Expand Down
53 changes: 36 additions & 17 deletions e2e/specs/guards-instances.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
[
Expand Down Expand Up @@ -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) {
Expand All @@ -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()
},
Expand All @@ -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()
},
Expand Down
60 changes: 32 additions & 28 deletions src/RouterView.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -43,7 +46,7 @@ export const RouterViewImpl = defineComponent({

const injectedRoute = inject(routeLocationKey)!
const depth = inject(viewDepthKey, 0)
const matchedRouteRef = computed(
const matchedRouteRef = computed<RouteLocationMatched | undefined>(
() => (props.route || injectedRoute).matched[depth]
)

Expand All @@ -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
Expand All @@ -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,
Expand All @@ -119,7 +123,7 @@ export const RouterViewImpl = defineComponent({
// pass the vnode to the slot as a prop.
// h and <component :is="..."> both accept vnodes
slots.default
? slots.default({ Component: component, route })
? slots.default({ Component: component, route, key })
: component
)
}
Expand Down

0 comments on commit c518750

Please sign in to comment.