Skip to content

Commit 98e1339

Browse files
committed
fix(vue): guard traceVueRouter cleanup + pin router-tracing edge cases
Guard the WeakMap delete in cleanup() so a stale HMR cleanup can't disable dedup for the active guards. Add coverage for the onError empty-name fallback and blocked-initial-navigation pageload naming, and assert the settled route name (not just call count) on the existing onError test.
1 parent 9dded39 commit 98e1339

3 files changed

Lines changed: 25 additions & 2 deletions

File tree

packages/vue/src/traceVueRouter.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ export function traceVueRouter(router: unknown): () => void {
127127
safeInvoke(offAfter);
128128
safeInvoke(offError);
129129
safeInvoke(() => nav.unregister());
130-
instrumented.delete(r);
130+
if (instrumented.get(r) === cleanup) instrumented.delete(r);
131131
};
132132
instrumented.set(r, cleanup);
133133
return cleanup;

packages/vue/tests/vue-router.integration.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ describe('traceVueRouter against a real vue-router', () => {
178178
await router.push('/product/p01').catch(() => {});
179179
expect(nav.startNavigation).toHaveBeenCalledTimes(1);
180180
expect(nav.settleNavigation).toHaveBeenCalledTimes(1);
181+
expect(nav.settleNavigation).toHaveBeenLastCalledWith({ name: '/', source: 'route' });
181182
});
182183

183184
it('names the pageload immediately when installed after the router is ready', async () => {

packages/vue/tests/vue-router.test.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import { traceVueRouter } from '../src/traceVueRouter';
3333
type Loc = { path: string; fullPath: string; matched: { path: string }[] };
3434
type Guard = (...a: any[]) => unknown;
3535

36-
function fakeRouter(current: Loc) {
36+
function fakeRouter(current: Loc | undefined) {
3737
let before: Guard[] = [];
3838
let after: Guard[] = [];
3939
let error: Guard[] = [];
@@ -59,13 +59,16 @@ function fakeRouter(current: Loc) {
5959
},
6060
fireBefore: (to: Loc, from: Loc) => before.forEach((g) => g(to, from)),
6161
fireAfter: (to: Loc, from: Loc, failure?: { type: number }) => after.forEach((g) => g(to, from, failure)),
62+
fireError: () => error.forEach((g) => g()),
6263
counts: () => ({ before: before.length, after: after.length, error: error.length }),
6364
};
6465
}
6566

6667
const home: Loc = { path: '/', fullPath: '/', matched: [{ path: '/' }] };
6768
const product: Loc = { path: '/product/p01', fullPath: '/product/p01', matched: [{ path: '/product/:id' }] };
6869
const cart: Loc = { path: '/cart', fullPath: '/cart', matched: [{ path: '/cart' }] };
70+
const blocked: Loc = { path: '/blocked', fullPath: '/blocked', matched: [{ path: '/blocked' }] };
71+
const START: Loc = { path: '/', fullPath: '/', matched: [] };
6972

7073
beforeEach(() => {
7174
nav.startNavigation.mockClear();
@@ -102,4 +105,23 @@ describe('traceVueRouter edge cases', () => {
102105
expect(router.counts()).toEqual({ before: 1, after: 1, error: 1 }); // prior removed, not doubled
103106
expect(nav.unregister).toHaveBeenCalledTimes(1); // prior nav source released
104107
});
108+
109+
it('falls back to an empty name on onError when there is no current route', () => {
110+
const router = fakeRouter(undefined); // currentRoute.value is undefined
111+
traceVueRouter(router); // must not throw on install-time enrichment either
112+
router.fireBefore(product, home); // client nav (home has matched → not initial): opens held root
113+
router.fireError();
114+
expect(nav.settleNavigation).toHaveBeenLastCalledWith({ name: '', source: 'url' });
115+
});
116+
117+
it('keeps a blocked initial navigation in pageload naming until the first success', () => {
118+
const router = fakeRouter(START); // START_LOCATION-like: empty matched → sawInitial stays false
119+
traceVueRouter(router);
120+
router.fireBefore(blocked, START); // treated as pageload naming, no nav root
121+
router.fireAfter(blocked, START, { type: 4 }); // aborted initial → still no nav root, sawInitial stays false
122+
router.fireBefore(product, START); // from is still START: aborted nav never committed
123+
router.fireAfter(product, START, undefined); // success → finalizes the pageload name
124+
expect(nav.startNavigation).not.toHaveBeenCalled();
125+
expect(nav.setActiveRouteName).toHaveBeenLastCalledWith({ name: '/product/:id', source: 'route' });
126+
});
105127
});

0 commit comments

Comments
 (0)