search

vuejs / router

🚦 The official router for Vue.js
https://router.vuejs.org/
MIT License
3.91k stars 1.19k forks source link

fix(guards): run beforeRouteEnter with app context #2053

Closed posva closed 8 months ago

posva commented 11 months ago

Fix vuejs/router#2051

netlify[bot] commented 11 months ago

Deploy Preview for vue-router canceled.

Name Link
Latest commit 66711f15dd2dee1132362e43d893e8355b2970a8
Latest deploy log https://app.netlify.com/sites/vue-router/deploys/655f1098d02aec00089382a3
posva commented 11 months ago

CI seems broken on the PR but works locally...

rijenkii commented 11 months ago

Modified packages/router/e2e/guards-instances/index.ts to output hasInjectionContext into logs and asynchronously loading view components, but it seems to fail:

Patch ```diff diff --git a/packages/router/e2e/guards-instances/index.ts b/packages/router/e2e/guards-instances/index.ts index 2a4aa04..8ff8ece 100644 --- a/packages/router/e2e/guards-instances/index.ts +++ b/packages/router/e2e/guards-instances/index.ts @@ -7,7 +7,14 @@ import { useRoute, useRouter, } from 'vue-router' -import { createApp, ref, reactive, defineComponent, computed } from 'vue' +import { + createApp, + ref, + reactive, + defineComponent, + computed, + hasInjectionContext, +} from 'vue' import { isArray } from '../../src/utils' // override existing style on dev with shorter times @@ -65,7 +72,11 @@ function createTestComponent(key: string) { // }, beforeRouteEnter(to, from, next) { state.enter++ - logs.value.push(`${key}: enter ${from.path} - ${to.path}`) + logs.value.push( + `${key}: enter ${from.path} - ${ + to.path + }, hasInjectionContext=${hasInjectionContext()}` + ) next(vm => { // @ts-expect-error vm.enterCallback++ @@ -75,21 +86,37 @@ function createTestComponent(key: string) { if (!this || this.key !== key) throw new Error('no this') state.update++ this.selfUpdates++ - logs.value.push(`${key}: update ${from.path} - ${to.path}`) + logs.value.push( + `${key}: update ${from.path} - ${ + to.path + }, hasInjectionContext=${hasInjectionContext()}` + ) }, beforeRouteLeave(to, from) { if (!this || this.key !== key) throw new Error('no this') state.leave++ this.selfLeaves++ - logs.value.push(`${key}: leave ${from.path} - ${to.path}`) + logs.value.push( + `${key}: leave ${from.path} - ${ + to.path + }, hasInjectionContext=${hasInjectionContext()}` + ) }, setup() { onBeforeRouteUpdate((to, from) => { - logs.value.push(`${key}: setup:update ${from.path} - ${to.path}`) + logs.value.push( + `${key}: setup:update ${from.path} - ${ + to.path + }, hasInjectionContext=${hasInjectionContext()}` + ) }) onBeforeRouteLeave((to, from) => { - logs.value.push(`${key}: setup:leave ${from.path} - ${to.path}`) + logs.value.push( + `${key}: setup:leave ${from.path} - ${ + to.path + }, hasInjectionContext=${hasInjectionContext()}` + ) }) return {} }, @@ -109,33 +136,33 @@ const webHistory = createWebHistory('/guards-instances') const router = createRouter({ history: webHistory, routes: [ - { path: '/', component: Home }, + { path: '/', component: async () => Home }, { path: '/foo', - component: Foo, + component: async () => Foo, }, { path: '/f/:id', - component: Foo, + component: async () => Foo, }, // TODO: test that the onBeforeRouteUpdate isn't kept { path: '/b/:id', name: 'id', - component: WithId, + component: async () => WithId, }, { path: '/named-one', components: { - default: One, - aux: Aux, + default: async () => One, + aux: async () => Aux, }, }, { path: '/named-two', components: { - default: Two, - aux: Aux, + default: async () => Two, + aux: async () => Aux, }, }, ], diff --git a/packages/router/e2e/specs/guards-instances.js b/packages/router/e2e/specs/guards-instances.js index b2394e4..35b5976 100644 --- a/packages/router/e2e/specs/guards-instances.js +++ b/packages/router/e2e/specs/guards-instances.js @@ -23,17 +23,17 @@ function testCase(browser, name) { .expect.element('#logs') .text.to.equal( [ - `${name}: enter / - /foo`, - `${name}: leave /foo - /f/1`, - `${name}: setup:leave /foo - /f/1`, - `${name}: enter /foo - /f/1`, - `${name}: update /f/1 - /f/2`, - `${name}: setup:update /f/1 - /f/2`, - `${name}: update /f/2 - /f/2`, - `${name}: setup:update /f/2 - /f/2`, - `${name}: leave /f/2 - /foo`, - `${name}: setup:leave /f/2 - /foo`, - `${name}: enter /f/2 - /foo`, + `${name}: enter / - /foo, hasInjectionContext=true`, + `${name}: leave /foo - /f/1, hasInjectionContext=true`, + `${name}: setup:leave /foo - /f/1, hasInjectionContext=true`, + `${name}: enter /foo - /f/1, hasInjectionContext=true`, + `${name}: update /f/1 - /f/2, hasInjectionContext=true`, + `${name}: setup:update /f/1 - /f/2, hasInjectionContext=true`, + `${name}: update /f/2 - /f/2, hasInjectionContext=true`, + `${name}: setup:update /f/2 - /f/2, hasInjectionContext=true`, + `${name}: leave /f/2 - /foo, hasInjectionContext=true`, + `${name}: setup:leave /f/2 - /foo, hasInjectionContext=true`, + `${name}: enter /f/2 - /foo, hasInjectionContext=true`, ].join('\n') ) } @@ -126,10 +126,10 @@ module.exports = { .expect.element('#logs') .text.to.equal( [ - `${name}: update /f/1 - /f/2`, - `${name}: setup:update /f/1 - /f/2`, - `${name}: leave /f/2 - /`, - `${name}: setup:leave /f/2 - /`, + `${name}: update /f/1 - /f/2, hasInjectionContext=true`, + `${name}: setup:update /f/1 - /f/2, hasInjectionContext=true`, + `${name}: leave /f/2 - /, hasInjectionContext=true`, + `${name}: setup:leave /f/2 - /, hasInjectionContext=true`, ].join('\n') ) @@ -175,8 +175,8 @@ module.exports = { .text.to.equal( [ // to force new lines formatting - `${name}: update /f/2 - /f/2`, - `${name}: setup:update /f/2 - /f/2`, + `${name}: update /f/2 - /f/2, hasInjectionContext=true`, + `${name}: setup:update /f/2 - /f/2, hasInjectionContext=true`, ].join('\n') ) browser @@ -238,8 +238,8 @@ module.exports = { .text.to.equal( [ // foo - `${name}: update /f/2 - /f/2`, - `${name}: setup:update /f/2 - /f/2`, + `${name}: update /f/2 - /f/2, hasInjectionContext=true`, + `${name}: setup:update /f/2 - /f/2, hasInjectionContext=true`, ].join('\n') ) browser @@ -250,10 +250,10 @@ module.exports = { .expect.element('#logs') .text.to.equal( [ - `${name}: update /f/2 - /f/2`, - `${name}: setup:update /f/2 - /f/2`, - `${name}: update /f/2 - /f/2`, - `${name}: setup:update /f/2 - /f/2`, + `${name}: update /f/2 - /f/2, hasInjectionContext=true`, + `${name}: setup:update /f/2 - /f/2, hasInjectionContext=true`, + `${name}: update /f/2 - /f/2, hasInjectionContext=true`, + `${name}: setup:update /f/2 - /f/2, hasInjectionContext=true`, ].join('\n') ) @@ -271,12 +271,12 @@ module.exports = { .expect.element('#logs') .text.to.equal( [ - `One: enter / - /named-one`, - `Aux: enter / - /named-one`, - `One: leave /named-one - /`, - `Aux: leave /named-one - /`, - `One: setup:leave /named-one - /`, - `Aux: setup:leave /named-one - /`, + `One: enter / - /named-one, hasInjectionContext=true`, + `Aux: enter / - /named-one, hasInjectionContext=true`, + `One: leave /named-one - /, hasInjectionContext=true`, + `Aux: leave /named-one - /, hasInjectionContext=true`, + `One: setup:leave /named-one - /, hasInjectionContext=true`, + `Aux: setup:leave /named-one - /, hasInjectionContext=true`, ].join('\n') ) @@ -287,12 +287,12 @@ module.exports = { .expect.element('#logs') .text.to.equal( [ - `One: leave /named-one - /named-two`, - `Aux: leave /named-one - /named-two`, - `One: setup:leave /named-one - /named-two`, - `Aux: setup:leave /named-one - /named-two`, - `Two: enter /named-one - /named-two`, - `Aux: enter /named-one - /named-two`, + `One: leave /named-one - /named-two, hasInjectionContext=true`, + `Aux: leave /named-one - /named-two, hasInjectionContext=true`, + `One: setup:leave /named-one - /named-two, hasInjectionContext=true`, + `Aux: setup:leave /named-one - /named-two, hasInjectionContext=true`, + `Two: enter /named-one - /named-two, hasInjectionContext=true`, + `Aux: enter /named-one - /named-two, hasInjectionContext=true`, ].join('\n') ) ```
pnpm run -r test:e2e output ``` │ ️TEST FAILURE (40.598s): │ - 1 assertions failed; 259 passed │ - 6 skipped │ ✖ 1) guards-instances │ – guards instances (5.739s) │ → ✖ NightwatchAssertError │ Expected element <#logs> text to equal: "Foo: enter / - /foo, hasInjectionContext=true │ Foo: leave /foo - /f/1, hasInjectionContext=true │ Foo: setup:leave /foo - /f/1, hasInjectionContext=true │ Foo: enter /foo - /f/1, hasInjectionContext=true │ Foo: update /f/1 - /f/2, hasInjectionContext=true │ Foo: setup:update /f/1 - /f/2, hasInjectionContext=true │ Foo: update /f/2 - /f/2, hasInjectionContext=true │ Foo: setup:update /f/2 - /f/2, hasInjectionContext=true │ Foo: leave /f/2 - /foo, hasInjectionContext=true │ Foo: setup:leave /f/2 - /foo, hasInjectionContext=true │ Foo: enter /f/2 - /foo, hasInjectionContext=true" - expected "equal 'Foo: enter / - /foo, hasInjectionContext=true │ Foo: leave /foo - /f/1, hasInjectionContext=true │ Foo: setup:leave /foo - /f/1, hasInjectionContext=true │ Foo: enter /foo - /f/1, hasInjectionContext=true │ Foo: update /f/1 - /f/2, hasInjectionContext=true │ Foo: setup:update /f/1 - /f/2, hasInjectionContext=true │ Foo: update /f/2 - /f/2, hasInjectionContext=true │ Foo: setup:update /f/2 - /f/2, hasInjectionContext=true │ Foo: leave /f/2 - /foo, hasInjectionContext=true │ Foo: setup:leave /f/2 - /foo, hasInjectionContext=true │ Foo: enter /f/2 - /foo, hasInjectionContext=true'" but got: "Foo: enter / - /foo, hasInjectionContext=false │ Foo: leave /foo - /f/1, hasInjectionContext=true │ Foo: setup:leave /foo - /f/1, hasInjectionContext=true │ Foo: enter /foo - /f/1, hasInjectionContext=false │ Foo: update /f/1 - /f/2, hasInjectionContext=true │ Foo: setup:update /f/1 - /f/2, hasInjectionContext=true │ Foo: update /f/2 - /f/2, hasInjectionContext=true │ Foo: setup:update /f/2 - /f/2, hasInjectionContext=true │ Foo: leave /f/2 - /foo, hasInjectionContext=true │ Foo: setup:leave /f/2 - /foo, hasInjectionContext=true │ Foo: enter /f/2 - /foo, hasInjectionContext=true" (5104ms) │ │ Error location: │ /tmp/router/packages/router/e2e/specs/guards-instances.js: │ ––––––––––––––––––––––––––––––––––––––––––––––––––––––––––– │ 21 | .assert.textContains(`#${name} .update`, '2') │ 22 | .assert.textContains(`#${name} .leave`, '2') │ 23 | .expect.element('#logs') │ 24 | .text.to.equal( │ 25 | [ │ ––––––––––––––––––––––––––––––––––––––––––––––––––––––––––– │ │ SKIPPED (at runtime): │ - cancel pending pop navigations │ - guards instances transition │ - guards instances keep alive │ - guards instances keyed │ - guards instances keepalive keyed │ - guards + instances + named views │ Wrote HTML report file to: /tmp/router/packages/router/tests_output/nightwatch-html-report/index.html │ │  ELIFECYCLE  Command failed with exit code 5. └─ Failed in 41.3s at /tmp/router/packages/router ```
posva commented 11 months ago

Thanks for the patch! We can go with a unit test instead (I pushed the failing test). What I added doesn't fix the issue.

Feel free to give it a try and even open a new PR based on this one. It doesn't matter if you keep the commits or not 🙂