Skip to content

Commit

Permalink
fix(vue): Don't call next in Vue router 4 instrumentation (#8351)
Browse files Browse the repository at this point in the history
Vue Router 4 no longer exposes a `next` resolver function to call inside a `beforeEach` [router guard callback](https://router.vuejs.org/guide/advanced/navigation-guards.html#global-before-guards).
Instead it will resolve the hook when the callback function returns.

This PR checks if `next` is available and only calls it then. Also it
adjusts the types accordingly and adds a test that previously failed.

fixes #8349
  • Loading branch information
Lms24 authored Jun 19, 2023
1 parent a4c858f commit e6ea537
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 3 deletions.
9 changes: 7 additions & 2 deletions packages/vue/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export type Route = {

interface VueRouter {
onError: (fn: (err: Error) => void) => void;
beforeEach: (fn: (to: Route, from: Route, next: () => void) => void) => void;
beforeEach: (fn: (to: Route, from: Route, next?: () => void) => void) => void;
}

/**
Expand Down Expand Up @@ -129,7 +129,12 @@ export function vueRouterInstrumentation(
});
}

next();
// Vue Router 4 no longer exposes the `next` function, so we need to
// check if it's available before calling it.
// `next` needs to be called in Vue Router 3 so that the hook is resolved.
if (next) {
next();
}
});
};
}
28 changes: 27 additions & 1 deletion packages/vue/test/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const captureExceptionSpy = jest.spyOn(SentryBrowser, 'captureException');

const mockVueRouter = {
onError: jest.fn<void, [(error: Error) => void]>(),
beforeEach: jest.fn<void, [(from: Route, to: Route, next: () => void) => void]>(),
beforeEach: jest.fn<void, [(from: Route, to: Route, next?: () => void) => void]>(),
};

const mockStartTransaction = jest.fn();
Expand Down Expand Up @@ -324,4 +324,30 @@ describe('vueRouterInstrumentation()', () => {
expect(mockStartTransaction).toHaveBeenCalledTimes(expectedCallsAmount + 1);
},
);

it("doesn't throw when `next` is not available in the beforeEach callback (Vue Router 4)", () => {
const instrument = vueRouterInstrumentation(mockVueRouter, { routeLabel: 'path' });
instrument(mockStartTransaction, true, true);
const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0];

const from = testRoutes.normalRoute1;
const to = testRoutes.namedRoute;
beforeEachCallback(to, from, undefined);

// first startTx call happens when the instrumentation is initialized (for pageloads)
expect(mockStartTransaction).toHaveBeenLastCalledWith({
name: '/login',
metadata: {
source: 'route',
},
data: {
params: to.params,
query: to.query,
},
op: 'navigation',
tags: {
'routing.instrumentation': 'vue-router',
},
});
});
});

0 comments on commit e6ea537

Please sign in to comment.