Skip to content

Commit aad8c73

Browse files
posvaposva
committed
refactor(router-view): use a key instead of watcher
1 parent 8c32738 commit aad8c73

File tree

3 files changed

+86
-48
lines changed

3 files changed

+86
-48
lines changed

‎e2e/guards-instances/index.ts

+18-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
import { createRouter, createWebHistory } from '../../src'
1+
import {
2+
createRouter,
3+
createWebHistory,
4+
onBeforeRouteUpdate,
5+
onBeforeRouteLeave,
6+
} from '../../src'
27
import { createApp, ref, reactive, defineComponent } from 'vue'
38

49
// override existing style on dev with shorter times
@@ -58,6 +63,16 @@ const Foo = defineComponent({
5863
state.leave++
5964
logs.value.push(`leave ${from.path} - ${to.path}`)
6065
},
66+
67+
setup() {
68+
onBeforeRouteUpdate((to, from) => {
69+
console.log(`setup update ${from.path} - ${to.path}`)
70+
})
71+
onBeforeRouteLeave((to, from) => {
72+
console.log(`setup leave ${from.path} - ${to.path}`)
73+
})
74+
return {}
75+
},
6176
})
6277

6378
const webHistory = createWebHistory('/' + __dirname)
@@ -121,9 +136,9 @@ leaves: {{ state.leave }}
121136
<router-view :key="$route.query.foo" class="view" />
122137
</template>
123138
<template v-else-if="testCase === 'keepalivekeyed'">
124-
<router-view v-slot="{ Component }" >
139+
<router-view v-slot="{ Component, key }" >
125140
<keep-alive>
126-
<component :is="Component" :key="$route.query.foo" class="view" />
141+
<component :is="Component" :key="$route.query.foo || key" class="view" />
127142
</keep-alive>
128143
</router-view>
129144
</template>

‎e2e/specs/guards-instances.js

+36-17
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@ function testCase(browser) {
55
.click('li:nth-child(2) a')
66
.assert.containsText('.view', 'foo 1')
77
.click('li:nth-child(3) a')
8-
.assert.containsText('.view', 'foo 2')
8+
.assert.containsText('.view', 'foo 1')
99
.click('li:nth-child(4) a')
10-
.assert.containsText('.view', 'foo 2')
10+
.assert.containsText('.view', 'foo 1')
1111
.click('li:nth-child(5) a')
12-
.assert.containsText('.view', 'foo 2')
12+
.assert.containsText('.view', 'foo 1')
1313
.click('li:nth-child(2) a')
14-
.assert.containsText('.view', 'foo 3')
14+
.assert.containsText('.view', 'foo')
1515
.assert.containsText(
1616
'#logs',
1717
[
@@ -64,23 +64,28 @@ module.exports = {
6464
.click('li:nth-child(1) a')
6565
// keep alive keeps the correct instance
6666
.click('li:nth-child(2) a')
67+
.assert.containsText('.view', 'foo 3')
68+
.click('li:nth-child(1) a')
69+
.click('li:nth-child(2) a')
6770
.assert.containsText('.view', 'foo 4')
71+
.click('li:nth-child(3) a')
72+
.assert.containsText('.view', 'foo 2')
6873

6974
browser.end()
7075
},
7176

72-
// /** @type {import('nightwatch').NightwatchTest} */
73-
// 'guards instances transition': function (browser) {
74-
// browser
75-
// .url('http://localhost:8080/guards-instances/')
76-
// .waitForElementPresent('#app > *', 1000)
77+
/** @type {import('nightwatch').NightwatchTest} */
78+
'guards instances transition': function (browser) {
79+
browser
80+
.url('http://localhost:8080/guards-instances/')
81+
.waitForElementPresent('#app > *', 1000)
7782

78-
// .click('#test-transition')
83+
.click('#test-transition')
7984

80-
// testCase(browser)
85+
testCase(browser)
8186

82-
// browser.end()
83-
// },
87+
browser.end()
88+
},
8489

8590
/** @type {import('nightwatch').NightwatchTest} */
8691
'guards instances keyed': function (browser) {
@@ -97,6 +102,13 @@ module.exports = {
97102
// the query is used as a key resetting the enter count
98103
.click('li:nth-child(6) a')
99104
.assert.containsText('.view', 'foo 0')
105+
// changing both the route and mounting the component
106+
.click('li:nth-child(2) a')
107+
.assert.containsText('.view', 'foo 1')
108+
.click('li:nth-child(6) a')
109+
.assert.containsText('.view', 'foo 1')
110+
.click('li:nth-child(2) a')
111+
.assert.containsText('.view', 'foo 1')
100112

101113
browser.end()
102114
},
@@ -112,18 +124,25 @@ module.exports = {
112124
testCase(browser)
113125

114126
browser
127+
.click('li:nth-child(1) a')
128+
// keep alive keeps the correct instance
129+
.click('li:nth-child(2) a')
130+
.assert.containsText('.view', 'foo 3')
131+
.click('li:nth-child(1) a')
132+
.click('li:nth-child(2) a')
133+
.assert.containsText('.view', 'foo 4')
134+
.click('li:nth-child(3) a')
135+
.assert.containsText('.view', 'foo 2')
136+
115137
.click('li:nth-child(5) a')
116138
// the query is used as a key resetting the enter count
117139
.click('li:nth-child(6) a')
118140
.assert.containsText('.view', 'foo 0')
119-
// going back to /foo
120-
.click('li:nth-child(2) a')
121-
.assert.containsText('.view', 'foo 5')
122141
.click('li:nth-child(1) a')
123142
.click('li:nth-child(6) a')
124143
.assert.containsText('.view', 'foo 1')
125144
.click('li:nth-child(5) a')
126-
.assert.containsText('.view', 'foo 5')
145+
.assert.containsText('.view', 'foo 2')
127146

128147
browser.end()
129148
},

‎src/RouterView.ts

+32-28
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,12 @@ import {
1111
computed,
1212
AllowedComponentProps,
1313
ComponentCustomProps,
14-
watch,
1514
} from 'vue'
16-
import { RouteLocationNormalized, RouteLocationNormalizedLoaded } from './types'
15+
import {
16+
RouteLocationNormalized,
17+
RouteLocationNormalizedLoaded,
18+
RouteLocationMatched,
19+
} from './types'
1720
import {
1821
matchedRouteKey,
1922
viewDepthKey,
@@ -43,7 +46,7 @@ export const RouterViewImpl = defineComponent({
4346

4447
const injectedRoute = inject(routeLocationKey)!
4548
const depth = inject(viewDepthKey, 0)
46-
const matchedRouteRef = computed(
49+
const matchedRouteRef = computed<RouteLocationMatched | undefined>(
4750
() => (props.route || injectedRoute).matched[depth]
4851
)
4952

@@ -55,35 +58,39 @@ export const RouterViewImpl = defineComponent({
5558
// when the same component is used in different routes, the onVnodeMounted
5659
// hook doesn't trigger, so we need to observe the changing route to update
5760
// the instance on the record
58-
watch(matchedRouteRef, to => {
59-
const currentName = props.name
60-
// to can be null if there isn't a matched route, e.g. not found
61-
if (to && !to.instances[currentName]) {
62-
to.instances[currentName] = viewRef.value
63-
// trigger enter callbacks when different routes only
64-
if (viewRef.value) {
65-
;(to.enterCallbacks[currentName] || []).forEach(callback =>
66-
callback(viewRef.value!)
67-
)
68-
// avoid double calls since watch is called before the onVnodeMounted
69-
to.enterCallbacks[currentName] = []
70-
}
71-
}
72-
})
61+
// watch(matchedRouteRef, to => {
62+
// const currentName = props.name
63+
// // to can be null if there isn't a matched route, e.g. not found
64+
// if (to && !to.instances[currentName]) {
65+
// to.instances[currentName] = viewRef.value
66+
// // trigger enter callbacks when different routes only
67+
// if (viewRef.value) {
68+
// ;(to.enterCallbacks[currentName] || []).forEach(callback =>
69+
// callback(viewRef.value!)
70+
// )
71+
// // avoid double calls since watch is called before the onVnodeMounted
72+
// to.enterCallbacks[currentName] = []
73+
// }
74+
// }
75+
// })
7376

7477
return () => {
7578
const route = props.route || injectedRoute
7679
const matchedRoute = matchedRouteRef.value
7780
const ViewComponent = matchedRoute && matchedRoute.components[props.name]
81+
// we need the value at the time we render because when we unmount, we
82+
// navigated to a different location so the value is different
83+
const currentName = props.name
84+
const key = matchedRoute && currentName + matchedRoute.path
7885

7986
if (!ViewComponent) {
8087
return slots.default
81-
? slots.default({ Component: ViewComponent, route })
88+
? slots.default({ Component: ViewComponent, route, key })
8289
: null
8390
}
8491

8592
// props from route configuration
86-
const routePropsOption = matchedRoute.props[props.name]
93+
const routePropsOption = matchedRoute!.props[props.name]
8794
const routeProps = routePropsOption
8895
? routePropsOption === true
8996
? route.params
@@ -92,23 +99,20 @@ export const RouterViewImpl = defineComponent({
9299
: routePropsOption
93100
: null
94101

95-
// we need the value at the time we render because when we unmount, we
96-
// navigated to a different location so the value is different
97-
const currentName = props.name
98102
const onVnodeMounted = () => {
99-
matchedRoute.instances[currentName] = viewRef.value
100-
;(matchedRoute.enterCallbacks[currentName] || []).forEach(callback =>
103+
matchedRoute!.instances[currentName] = viewRef.value
104+
;(matchedRoute!.enterCallbacks[currentName] || []).forEach(callback =>
101105
callback(viewRef.value!)
102106
)
103107
}
104108
const onVnodeUnmounted = () => {
105109
// remove the instance reference to prevent leak
106-
matchedRoute.instances[currentName] = null
110+
matchedRoute!.instances[currentName] = null
107111
}
108112

109113
const component = h(
110114
ViewComponent,
111-
assign({}, routeProps, attrs, {
115+
assign({ key }, routeProps, attrs, {
112116
onVnodeMounted,
113117
onVnodeUnmounted,
114118
ref: viewRef,
@@ -119,7 +123,7 @@ export const RouterViewImpl = defineComponent({
119123
// pass the vnode to the slot as a prop.
120124
// h and <component :is="..."> both accept vnodes
121125
slots.default
122-
? slots.default({ Component: component, route })
126+
? slots.default({ Component: component, route, key })
123127
: component
124128
)
125129
}

0 commit comments

Comments
 (0)