diff --git a/packages/vue/src/router.ts b/packages/vue/src/router.ts index 2506fd220207..8d4100e9b6c8 100644 --- a/packages/vue/src/router.ts +++ b/packages/vue/src/router.ts @@ -30,6 +30,9 @@ export type Route = { interface VueRouter { onError: (fn: (err: Error) => void) => void; beforeEach: (fn: (to: Route, from: Route, next?: () => void) => void) => void; + // Vue Router 3 exposes a `mode` property ('hash' | 'history' | 'abstract'). + // Vue Router 4+ replaced it with `options.history`. Used for version detection. + mode?: string; } /** @@ -52,9 +55,16 @@ export function instrumentVueRouter( ): void { let hasHandledFirstPageLoad = false; + // Detect Vue Router 3 by checking for the `mode` property which only exists in VR3. + // Vue Router 4+ uses `options.history` instead and does not expose `mode`. + const isLegacyRouter = 'mode' in router; + router.onError(error => captureException(error, { mechanism: { handled: false } })); - router.beforeEach((to, _from, next) => { + // Use rest params to capture `next` without declaring it as a named parameter. + // This keeps Function.length === 2, which tells Vue Router 4+/5+ to use the + // modern return-based resolution (no deprecation warning in Vue Router 5.0.3+). + router.beforeEach((to: Route, _from: Route, ...rest: [(() => void)?]) => { // We avoid trying to re-fetch the page load span when we know we already handled it the first time const activePageLoadSpan = !hasHandledFirstPageLoad ? getActivePageLoadSpan() : undefined; @@ -116,11 +126,16 @@ export function instrumentVueRouter( }); } - // 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(); + // Vue Router 3 requires `next()` to be called to resolve the navigation guard. + // Vue Router 4+ auto-resolves guards with Function.length < 3 via `guardToPromiseFn`. + // In Vue Router 5.0.3+, the `next` callback passed to guards is wrapped with + // `withDeprecationWarning()`, so calling it emits a console warning. We avoid + // calling it on modern routers where it is both unnecessary and noisy. + if (isLegacyRouter) { + const next = rest[0]; + if (typeof next === 'function') { + next(); + } } }); } diff --git a/packages/vue/test/router.test.ts b/packages/vue/test/router.test.ts index 55dcff7bc25b..763f3e51d6f7 100644 --- a/packages/vue/test/router.test.ts +++ b/packages/vue/test/router.test.ts @@ -23,11 +23,9 @@ vi.mock('@sentry/core', async () => { const mockVueRouter = { onError: vi.fn<[(error: Error) => void]>(), - beforeEach: vi.fn<[(from: Route, to: Route, next?: () => void) => void]>(), + beforeEach: vi.fn<[(from: Route, to: Route) => void]>(), }; -const mockNext = vi.fn(); - const testRoutes: Record = { initialPageloadRoute: { matched: [], params: {}, path: '', query: {} }, normalRoute1: { @@ -118,8 +116,8 @@ describe('instrumentVueRouter()', () => { const from = testRoutes[fromKey]!; const to = testRoutes[toKey]!; - beforeEachCallback(to, testRoutes['initialPageloadRoute']!, mockNext); // fake initial pageload - beforeEachCallback(to, from, mockNext); + beforeEachCallback(to, testRoutes['initialPageloadRoute']!); // fake initial pageload + beforeEachCallback(to, from); expect(mockStartSpan).toHaveBeenCalledTimes(2); expect(mockStartSpan).toHaveBeenLastCalledWith({ @@ -131,8 +129,6 @@ describe('instrumentVueRouter()', () => { }, op: 'navigation', }); - - expect(mockNext).toHaveBeenCalledTimes(2); }, ); @@ -171,7 +167,7 @@ describe('instrumentVueRouter()', () => { const from = testRoutes[fromKey]!; const to = testRoutes[toKey]!; - beforeEachCallback(to, from, mockNext); + beforeEachCallback(to, from); expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); expect(mockRootSpan.updateName).toHaveBeenCalledWith(transactionName); @@ -180,8 +176,6 @@ describe('instrumentVueRouter()', () => { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.vue', ...getAttributesForRoute(to), }); - - expect(mockNext).toHaveBeenCalledTimes(1); }, ); @@ -198,8 +192,8 @@ describe('instrumentVueRouter()', () => { const from = testRoutes.normalRoute1!; const to = testRoutes.namedRoute!; - beforeEachCallback(to, testRoutes['initialPageloadRoute']!, mockNext); // fake initial pageload - beforeEachCallback(to, from, mockNext); + beforeEachCallback(to, testRoutes['initialPageloadRoute']!); // fake initial pageload + beforeEachCallback(to, from); // first startTx call happens when the instrumentation is initialized (for pageloads) expect(mockStartSpan).toHaveBeenLastCalledWith({ @@ -226,8 +220,8 @@ describe('instrumentVueRouter()', () => { const from = testRoutes.normalRoute1!; const to = testRoutes.namedRoute!; - beforeEachCallback(to, testRoutes['initialPageloadRoute']!, mockNext); // fake initial pageload - beforeEachCallback(to, from, mockNext); + beforeEachCallback(to, testRoutes['initialPageloadRoute']!); // fake initial pageload + beforeEachCallback(to, from); // first startTx call happens when the instrumentation is initialized (for pageloads) expect(mockStartSpan).toHaveBeenLastCalledWith({ @@ -284,7 +278,7 @@ describe('instrumentVueRouter()', () => { const to = testRoutes['normalRoute1']!; const from = testRoutes['initialPageloadRoute']!; - beforeEachCallback(to, from, mockNext); + beforeEachCallback(to, from); expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); @@ -318,7 +312,7 @@ describe('instrumentVueRouter()', () => { const from = testRoutes['initialPageloadRoute']!; const to = testRoutes['normalRoute1']!; - beforeEachCallback(to, from, mockNext); + beforeEachCallback(to, from); expect(scopeSetTransactionNameSpy).toHaveBeenCalledTimes(1); expect(scopeSetTransactionNameSpy).toHaveBeenCalledWith('/books/:bookId/chapter/:chapterId'); @@ -357,7 +351,7 @@ describe('instrumentVueRouter()', () => { expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0]![0]!; - beforeEachCallback(testRoutes['normalRoute1']!, testRoutes['initialPageloadRoute']!, mockNext); + beforeEachCallback(testRoutes['normalRoute1']!, testRoutes['initialPageloadRoute']!); expect(mockRootSpan.updateName).toHaveBeenCalledTimes(expectedCallsAmount); expect(mockStartSpan).not.toHaveBeenCalled(); @@ -381,14 +375,14 @@ describe('instrumentVueRouter()', () => { expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0]![0]!; - beforeEachCallback(testRoutes['normalRoute1']!, testRoutes['initialPageloadRoute']!, mockNext); // fake initial pageload - beforeEachCallback(testRoutes['normalRoute2']!, testRoutes['normalRoute1']!, mockNext); + beforeEachCallback(testRoutes['normalRoute1']!, testRoutes['initialPageloadRoute']!); // fake initial pageload + beforeEachCallback(testRoutes['normalRoute2']!, testRoutes['normalRoute1']!); expect(mockStartSpan).toHaveBeenCalledTimes(expectedCallsAmount); }, ); - it("doesn't throw when `next` is not available in the beforeEach callback (Vue Router 4)", () => { + it('does not declare a third parameter to avoid Vue Router next() deprecation warning', () => { const mockStartSpan = vi.fn().mockReturnValue(MOCK_SPAN); instrumentVueRouter( mockVueRouter, @@ -398,21 +392,45 @@ describe('instrumentVueRouter()', () => { const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0]![0]!; - const from = testRoutes.normalRoute1!; - const to = testRoutes.namedRoute!; - beforeEachCallback(to, testRoutes['initialPageloadRoute']!, mockNext); // fake initial pageload - beforeEachCallback(to, from, undefined); + // Vue Router uses Function.length to detect whether the guard uses the legacy + // `next` callback. Guards with < 3 params use the modern return-based pattern. + expect(beforeEachCallback.length).toBeLessThan(3); + }); - // first startTx call happens when the instrumentation is initialized (for pageloads) - expect(mockStartSpan).toHaveBeenLastCalledWith({ - name: '/login', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.vue', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - ...getAttributesForRoute(to), - }, - op: 'navigation', - }); + it('calls next() for Vue Router 3 (legacy router with mode property)', () => { + const mockNext = vi.fn(); + const mockLegacyRouter = { + onError: vi.fn<[(error: Error) => void]>(), + beforeEach: vi.fn<[(from: Route, to: Route, next?: () => void) => void]>(), + mode: 'history', + }; + + const mockStartSpan = vi.fn().mockReturnValue(MOCK_SPAN); + instrumentVueRouter( + mockLegacyRouter, + { routeLabel: 'name', instrumentPageLoad: true, instrumentNavigation: true }, + mockStartSpan, + ); + + const beforeEachCallback = mockLegacyRouter.beforeEach.mock.calls[0]![0]!; + beforeEachCallback(testRoutes['normalRoute1']!, testRoutes['initialPageloadRoute']!, mockNext); + + expect(mockNext).toHaveBeenCalledTimes(1); + }); + + it('does not call next() for Vue Router 4+ (modern router without mode property)', () => { + const mockNext = vi.fn(); + const mockStartSpan = vi.fn().mockReturnValue(MOCK_SPAN); + instrumentVueRouter( + mockVueRouter, + { routeLabel: 'name', instrumentPageLoad: true, instrumentNavigation: true }, + mockStartSpan, + ); + + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0]![0]!; + beforeEachCallback(testRoutes['normalRoute1']!, testRoutes['initialPageloadRoute']!, mockNext); + + expect(mockNext).not.toHaveBeenCalled(); }); });