-
Notifications
You must be signed in to change notification settings - Fork 148
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
⚗[REPLAY-465] Base tag support (feature flagged): Remove URL transformation from relative to absolute #1106
⚗[REPLAY-465] Base tag support (feature flagged): Remove URL transformation from relative to absolute #1106
Conversation
…on-feature-flagged
@@ -58,6 +64,9 @@ export function makeSrcsetUrlsAbsolute(attributeValue: string, baseUrl: string) | |||
|
|||
export function makeUrlAbsolute(url: string, baseUrl: string): string { | |||
try { | |||
if (isExperimentalFeatureEnabled('base-tag')) { | |||
return url.trim() |
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.
Do you need the .trim()
here? I'd suggest removing it.
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 was pondering that myself. Why did we trim in the first place? Updated in the meantime.
Codecov Report
@@ Coverage Diff @@
## main #1106 +/- ##
==========================================
- Coverage 89.07% 89.04% -0.04%
==========================================
Files 91 91
Lines 4193 4199 +6
Branches 956 959 +3
==========================================
+ Hits 3735 3739 +4
- Misses 458 460 +2
Continue to review full report at Codecov.
|
Motivation
As per internal RFC "Supporting Replay of Relative URLs When Tag Used", we want to support the tag. In short, this tag will cause the document to resolve relative URLs with the provided URL instead.
Changes
We don't need to transform URLs from relative to absolute on the SDK recorder side. Instead we will handle this on the replay side.
Testing
Removes tests for code that transforms URLs. Manual testing for now behind a feature flag.
I have gone over the contributing documentation.