-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(nuxt): Detect pageload by adding flag in Vue router #13171
Conversation
/* Make sure to import from '@nuxt/test-utils/playwright' in the tests | ||
* Like this: import { expect, test } from '@nuxt/test-utils/playwright' */ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this just as a heads-up in case someone adds new tests later on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable to me!
packages/vue/src/router.ts
Outdated
@@ -50,18 +50,27 @@ export function instrumentVueRouter( | |||
}, | |||
startNavigationSpanFn: (context: StartSpanOptions) => void, | |||
): void { | |||
let IS_FIRST_PAGE_LOAD = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: Given that this is not a constant, I'd say we don't capitalize the variable name
size-limit report 📦
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
@@ -26,8 +28,8 @@ export function init(options: SentryNuxtOptions): Client | undefined { | |||
// todo: the buildAssetDir could be changed in the nuxt config - change this to a more generic solution | |||
if (event.transaction?.match(/^GET \/_nuxt\//)) { | |||
options.debug && | |||
// eslint-disable-next-line no-console | |||
console.log('[Sentry] NuxtLowQualityTransactionsFilter filtered transaction: ', event.transaction); | |||
DEBUG_BUILD && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙏 good change!
Nuxt is using the Vue router under the hood, but the previous condition to detect a page load (
from.name == null && from.matched.length === 0
) does not work with Nuxt, asfrom.matched
is never empty.