Skip to content

Commit 6554171

Browse files
authored
fix(router-view): reuse saved instances in different records (vuejs#446)
BREAKING CHANGE: `onBeforeRouteLeave` and `onBeforeRouteUpdate` used to have access to the component instance through `instance.proxy` but given that: 1. It has been marked as `internal` (vuejs/core#1849) 2. When using `setup`, all variables are accessible on the scope (and should be accessed that way because the code minimizes better) It has been removed to prevent wrong usage and lighten Vue Router
1 parent 6d6d454 commit 6554171

File tree

10 files changed

+543
-151
lines changed

10 files changed

+543
-151
lines changed

__tests__/guards/onBeforeRouteLeave.spec.ts

Lines changed: 1 addition & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -13,58 +13,10 @@ const component = {
1313
}
1414

1515
describe('onBeforeRouteLeave', () => {
16-
it('invokes with the component context', async () => {
17-
expect.assertions(2)
18-
const spy = jest
19-
.fn()
20-
.mockImplementationOnce(function (this: any, to, from, next) {
21-
expect(typeof this.counter).toBe('number')
22-
next()
23-
})
24-
const WithLeave = defineComponent({
25-
template: `text`,
26-
// we use data to check if the context is the right one because saving `this` in a variable logs a few warnings
27-
data: () => ({ counter: 0 }),
28-
setup() {
29-
onBeforeRouteLeave(spy)
30-
},
31-
})
32-
33-
const router = createRouter({
34-
history: createMemoryHistory(),
35-
routes: [
36-
{ path: '/', component },
37-
{ path: '/leave', component: WithLeave as any },
38-
],
39-
})
40-
const app = createApp({
41-
template: `
42-
<router-view />
43-
`,
44-
})
45-
app.use(router)
46-
const rootEl = document.createElement('div')
47-
document.body.appendChild(rootEl)
48-
app.mount(rootEl)
49-
50-
await router.isReady()
51-
await router.push('/leave')
52-
await router.push('/')
53-
expect(spy).toHaveBeenCalledTimes(1)
54-
})
55-
5616
it('removes guards when leaving the route', async () => {
57-
expect.assertions(3)
58-
const spy = jest
59-
.fn()
60-
.mockImplementation(function (this: any, to, from, next) {
61-
expect(typeof this.counter).toBe('number')
62-
next()
63-
})
17+
const spy = jest.fn()
6418
const WithLeave = defineComponent({
6519
template: `text`,
66-
// we use data to check if the context is the right one because saving `this` in a variable logs a few warnings
67-
data: () => ({ counter: 0 }),
6820
setup() {
6921
onBeforeRouteLeave(spy)
7022
},

__tests__/guards/onBeforeRouteUpdate.spec.ts

Lines changed: 1 addition & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -13,58 +13,10 @@ const component = {
1313
}
1414

1515
describe('onBeforeRouteUpdate', () => {
16-
it('invokes with the component context', async () => {
17-
expect.assertions(2)
18-
const spy = jest
19-
.fn()
20-
.mockImplementationOnce(function (this: any, to, from, next) {
21-
expect(typeof this.counter).toBe('number')
22-
next()
23-
})
24-
const WithLeave = defineComponent({
25-
template: `text`,
26-
// we use data to check if the context is the right one because saving `this` in a variable logs a few warnings
27-
data: () => ({ counter: 0 }),
28-
setup() {
29-
onBeforeRouteUpdate(spy)
30-
},
31-
})
32-
33-
const router = createRouter({
34-
history: createMemoryHistory(),
35-
routes: [
36-
{ path: '/', component },
37-
{ path: '/foo', component: WithLeave as any },
38-
],
39-
})
40-
const app = createApp({
41-
template: `
42-
<router-view />
43-
`,
44-
})
45-
app.use(router)
46-
const rootEl = document.createElement('div')
47-
document.body.appendChild(rootEl)
48-
app.mount(rootEl)
49-
50-
await router.isReady()
51-
await router.push('/foo')
52-
await router.push('/foo?q')
53-
expect(spy).toHaveBeenCalledTimes(1)
54-
})
55-
5616
it('removes update guards when leaving', async () => {
57-
expect.assertions(3)
58-
const spy = jest
59-
.fn()
60-
.mockImplementation(function (this: any, to, from, next) {
61-
expect(typeof this.counter).toBe('number')
62-
next()
63-
})
17+
const spy = jest.fn()
6418
const WithLeave = defineComponent({
6519
template: `text`,
66-
// we use data to check if the context is the right one because saving `this` in a variable logs a few warnings
67-
data: () => ({ counter: 0 }),
6820
setup() {
6921
onBeforeRouteUpdate(spy)
7022
},

e2e/guards-instances/index.html

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<!DOCTYPE html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="UTF-8" />
5+
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
6+
<meta http-equiv="X-UA-Compatible" content="ie=edge" />
7+
<title>Vue Router e2e tests - instances in guards</title>
8+
<!-- TODO: replace with local imports for promises and anything else needed -->
9+
<script src="https://polyfill.io/v3/polyfill.min.js?features=default%2Ces2015"></script>
10+
<style>
11+
.fade-enter-active,
12+
.fade-leave-active {
13+
transition: opacity 2s ease;
14+
}
15+
.fade-enter-from,
16+
.fade-leave-active {
17+
opacity: 0;
18+
}
19+
</style>
20+
</head>
21+
<body>
22+
<a href="/">&lt;&lt; Back to Homepage</a>
23+
<hr />
24+
25+
<main id="app"></main>
26+
</body>
27+
</html>

e2e/guards-instances/index.ts

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
import {
2+
createRouter,
3+
createWebHistory,
4+
onBeforeRouteUpdate,
5+
onBeforeRouteLeave,
6+
useRoute,
7+
useRouter,
8+
} from '../../src'
9+
import { createApp, ref, reactive, defineComponent, computed } from 'vue'
10+
11+
// override existing style on dev with shorter times
12+
if (!__CI__) {
13+
const transitionDuration = '0.5s'
14+
const styleEl = document.createElement('style')
15+
styleEl.innerHTML = `
16+
.fade-enter-active,
17+
.fade-leave-active {
18+
transition: opacity ${transitionDuration} ease;
19+
}
20+
.child-view {
21+
position: absolute;
22+
transition: all ${transitionDuration} cubic-bezier(0.55, 0, 0.1, 1);
23+
}
24+
`
25+
document.head.append(styleEl)
26+
}
27+
28+
const Home = defineComponent({
29+
template: `
30+
<div>
31+
<h2>Home</h2>
32+
</div>
33+
`,
34+
})
35+
36+
const logs = ref<string[]>([])
37+
38+
const state = reactive({
39+
enter: 0,
40+
update: 0,
41+
leave: 0,
42+
})
43+
44+
const Foo = defineComponent({
45+
template: `
46+
<div>
47+
foo
48+
<p id="enterCbs">{{ enterCallback }}</p>
49+
<p id="update">{{ selfUpdates }}</p>
50+
<p id="leave">{{ selfLeaves }}</p>
51+
</div>
52+
`,
53+
data: () => ({ key: 'Foo', enterCallback: 0, selfUpdates: 0, selfLeaves: 0 }),
54+
// mounted() {
55+
// console.log('mounted Foo')
56+
// },
57+
beforeRouteEnter(to, from, next) {
58+
state.enter++
59+
logs.value.push(`enter ${from.path} - ${to.path}`)
60+
next(vm => {
61+
// @ts-ignore
62+
vm.enterCallback++
63+
})
64+
},
65+
beforeRouteUpdate(to, from) {
66+
if (!this || this.key !== 'Foo') throw new Error('no this')
67+
state.update++
68+
this.selfUpdates++
69+
logs.value.push(`update ${from.path} - ${to.path}`)
70+
},
71+
beforeRouteLeave(to, from) {
72+
if (!this || this.key !== 'Foo') throw new Error('no this')
73+
state.leave++
74+
this.selfLeaves++
75+
logs.value.push(`leave ${from.path} - ${to.path}`)
76+
},
77+
78+
setup() {
79+
onBeforeRouteUpdate((to, from) => {
80+
logs.value.push(`setup:update ${from.path} - ${to.path}`)
81+
})
82+
onBeforeRouteLeave((to, from) => {
83+
logs.value.push(`setup:leave ${from.path} - ${to.path}`)
84+
})
85+
return {}
86+
},
87+
})
88+
89+
const webHistory = createWebHistory('/' + __dirname)
90+
const router = createRouter({
91+
history: webHistory,
92+
routes: [
93+
{ path: '/', component: Home },
94+
{
95+
path: '/foo',
96+
component: Foo,
97+
},
98+
{
99+
path: '/f/:id',
100+
component: Foo,
101+
},
102+
],
103+
})
104+
105+
// preserve existing query
106+
const originalPush = router.push
107+
router.push = to => {
108+
if (typeof to === 'string') {
109+
const resolved = router.resolve(to)
110+
return router.push({
111+
path: to,
112+
query: {
113+
testCase: router.currentRoute.value.query.testCase,
114+
...resolved.query,
115+
},
116+
})
117+
} else {
118+
return originalPush({
119+
...to,
120+
query: {
121+
testCase: router.currentRoute.value.query.testCase,
122+
...to.query,
123+
},
124+
})
125+
}
126+
}
127+
128+
const app = createApp({
129+
template: `
130+
<div id="app">
131+
<h1>Instances</h1>
132+
<p>Using {{ testCase || 'default' }}</p>
133+
<button id="test-normal" @click="testCase = ''">Use Normal</button>
134+
<button id="test-keepalive" @click="testCase = 'keepalive'">Use Keep Alive</button>
135+
<button id="test-transition" @click="testCase = 'transition'">Use Transition</button>
136+
<button id="test-keyed" @click="testCase = 'keyed'">Use keyed</button>
137+
<button id="test-keepalivekeyed" @click="testCase = 'keepalivekeyed'">Use Keep Alive Keyed</button>
138+
<pre>
139+
route: {{ $route.fullPath }}
140+
enters: {{ state.enter }}
141+
updates: {{ state.update }}
142+
leaves: {{ state.leave }}
143+
</pre>
144+
<pre id="logs">{{ logs.join('\\n') }}</pre>
145+
<button id="resetLogs" @click="logs = []">Reset Logs</button>
146+
<ul>
147+
<li><router-link to="/">/</router-link></li>
148+
<li><router-link to="/foo">/foo</router-link></li>
149+
<li><router-link to="/f/1">/f/1</router-link></li>
150+
<li><router-link to="/f/2">/f/2</router-link></li>
151+
<li><router-link to="/f/2?bar=foo">/f/2?bar=foo</router-link></li>
152+
<li><router-link to="/f/2?foo=key">/f/2?foo=key</router-link></li>
153+
<li><router-link to="/f/2?foo=key2">/f/2?foo=key2</router-link></li>
154+
</ul>
155+
156+
<template v-if="testCase === 'keepalive'">
157+
<router-view v-slot="{ Component }" >
158+
<keep-alive>
159+
<component :is="Component" class="view" />
160+
</keep-alive>
161+
</router-view>
162+
</template>
163+
<template v-else-if="testCase === 'transition'">
164+
<router-view v-slot="{ Component }" >
165+
<transition name="fade" mode="">
166+
<component :is="Component" class="view" />
167+
</transition>
168+
</router-view>
169+
</template>
170+
<template v-else-if="testCase === 'keyed'">
171+
<router-view :key="$route.query.foo" class="view" />
172+
</template>
173+
<template v-else-if="testCase === 'keepalivekeyed'">
174+
<router-view v-slot="{ Component }" >
175+
<keep-alive>
176+
<component :is="Component" :key="$route.query.foo" class="view" />
177+
</keep-alive>
178+
</router-view>
179+
</template>
180+
<template v-else>
181+
<router-view class="view" />
182+
</template>
183+
184+
</div>
185+
`,
186+
setup() {
187+
const router = useRouter()
188+
const route = useRoute()
189+
190+
const testCase = computed<string>({
191+
get: () => {
192+
let { testCase } = route.query
193+
return !testCase || Array.isArray(testCase) ? '' : testCase
194+
},
195+
set(testCase) {
196+
router.push({ query: { ...route.query, testCase } })
197+
},
198+
})
199+
200+
return { state, logs, testCase }
201+
},
202+
})
203+
204+
app.use(router)
205+
206+
app.mount('#app')

e2e/multi-app/index.ts

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { createRouter, createWebHistory, onBeforeRouteUpdate } from '../../src'
1+
import { createRouter, createWebHistory } from '../../src'
22
import { RouteComponent } from '../../src/types'
33
import { createApp, ref, watchEffect, App, inject } from 'vue'
44

@@ -26,15 +26,6 @@ const User: RouteComponent = {
2626
setup() {
2727
const id = inject('id')!
2828

29-
if (id !== 1)
30-
onBeforeRouteUpdate(function (to, from, next) {
31-
// @ts-ignore
32-
console.log('update from ', id, this.id)
33-
// @ts-ignore
34-
// this.count++
35-
next()
36-
})
37-
3829
return { id }
3930
},
4031
}

0 commit comments

Comments
 (0)