Conversation
| // afterEach fires for the initial navigation when the app is mounted via app.use(router). | ||
| // In tests without mounting, an explicit router.push() is needed to trigger the hook. | ||
| router.afterEach((to, _from, failure) => { | ||
| if (failure && !isNavigationFailure(failure, NavigationFailureType.duplicated)) { |
There was a problem hiding this comment.
Vue Router's afterEach fires even for navigations blocked by a guard, not just successful ones. The failure parameter tells you why. We skip cancelled (preempted by another navigation) and aborted (guard explicitly blocked it). duplicated (same route you're already on) we let through - that's not really a failure, the URL just didn't change because it was already there.
87699a2 to
fd5fb81
Compare
c870b10 to
9a8f915
Compare
Bundles Sizes Evolution
🚀 CPU Performance
🧠 Memory Performance
|
fd5fb81 to
9de709a
Compare
9a8f915 to
9a4e80e
Compare
9de709a to
95cd15e
Compare
9a4e80e to
4e0030b
Compare
|
✨ Fix all issues with BitsAI or with Cursor
|
3a8e944 to
8c710e0
Compare
95cd15e to
d87a4b9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ce5ece0d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }) | ||
| } | ||
|
|
||
| export function computeViewName(matched: RouteLocationMatched[]): string { |
There was a problem hiding this comment.
❓ question : don't we need a logic to resolve "catch-all" pattern like in react with this function ?
| import { onVueInit } from '../vuePlugin' | ||
|
|
||
| export function startVueRouterView(matched: RouteLocationMatched[]) { | ||
| onVueInit((configuration, rumPublicApi) => { |
There was a problem hiding this comment.
💬 suggestion: For consistency with React, onVueInit could be renamed to onRumInit. The same likely applies to onVueStart.
| }) | ||
| } | ||
|
|
||
| export function computeViewName(matched: RouteLocationMatched[]): string { |
There was a problem hiding this comment.
💬 suggestion: I think we are missing a case for catch all routes(aka wildcard). For those, in React we keep the path with the params.
| }) | ||
| }) | ||
|
|
||
| describe('computeViewName', () => { |
There was a problem hiding this comment.
🥜 nitpick: It would be great to align how these tests are written across integrations. I found, using an array of cases, like in the React tests, works well.
I tried to apply the same approach for Angular. Wdyt?
7693561 to
aaa50ac
Compare
aaa50ac to
90c0728
Compare
Motivation
Third of five PRs. #4325 and #4327 are already merged. After this, navigating between routes automatically creates RUM views with parameterized names instead of raw URLs.
Changes
Exports a wrapped
createRouterfrom@datadog/browser-rum-vue/vue-router-v4as a drop-in for the standardvue-routerimport:On each navigation,
afterEachcallsstartView()with the parameterized route name (/users/:idrather than/users/42). There is one non-obvious bit:afterEachalso fires for navigations that get blocked by a guard, so we filter those out. Details in a comment on the diff.Requires
vuePlugin({ router: true })alongside this import. That setstrackViewsManually: trueat init so the SDK's built-in URL tracking does not race with the router integration.Test instructions
Checklist