-
Notifications
You must be signed in to change notification settings - Fork 142
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
[RUMF-625] make sure view url doesn't change #469
Conversation
This commit makes withFakeLocation simpler to use: pass a base URL (or path) and the whole location will be built automatically. Also, it allows the test to change the fake location by using the history api.
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.
LGTM !
packages/rum/src/rum.ts
Outdated
@@ -359,6 +360,7 @@ function trackView(lifeCycle: LifeCycle, handler: (startTime: number, event: Rum | |||
loadingTime: view.loadingTime ? msToNs(view.loadingTime) : undefined, | |||
loadingType: view.loadingType, | |||
measures: view.measures, | |||
url: view.location.href, |
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.
With this approach, url from ViewContext is still wrong so it can still be wrongly associated to other event types.
What about fixing it there?
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.
I wrote a new fix, let me know if it looks right :)
Codecov Report
@@ Coverage Diff @@
## master #469 +/- ##
==========================================
+ Coverage 87.57% 87.60% +0.03%
==========================================
Files 32 32
Lines 2020 2025 +5
Branches 409 409
==========================================
+ Hits 1769 1774 +5
Misses 251 251
Continue to review full report at Codecov.
|
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.
👍
Motivation
When a SPA changes its URL, the ending view gets updated with the new URL. It should keep its initial URL no matter what.
See https://datadoghq.atlassian.net/browse/RUMF-625
Changes
When formating a RumViewEvent, use the one from the collected View object instead of the ViewContext which already contains the new URL
Testing
I have gone over the contributing documentation.