feat(pageview): set utm manually #618
Conversation
dosubot
bot
added
the
size:M
WalkthroughThe changes introduce several new utility functions for handling UTM parameters and URL manipulation. The Assessment against linked issues
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/api/pageview/pageview.test.ts(1 hunks)src/api/pageview/pageview.ts(2 hunks)src/api/set/set.test.ts(1 hunks)src/api/set/set.ts(1 hunks)src/api/time/time.test.ts(1 hunks)src/api/time/time.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/api/pageview/pageview.test.ts (1)
src/api/pageview/pageview.ts (1)
pageview(17-63)
🪛 GitHub Check: build
src/api/pageview/pageview.test.ts
[failure] 130-130: src/api/pageview/pageview.test.ts > pageview > should send utm parameters manually outside the page_view event
AssertionError: expected 1st "query" call to have been called with [ 'set', 'utm_tracking', { …(3) } ]
- Expected
-
Received
[
"set",
- "utm_tracking",
- "campaign",
{
"utm_campaign": "summer_sale",
"utm_medium": "cpc",
"utm_source": "google",
},
]
❯ src/api/pageview/pageview.test.ts:130:19
🪛 GitHub Actions: CI
src/api/pageview/pageview.test.ts
[error] 130-130: AssertionError: expected 1st 'query' call to have been called with [ 'set', 'utm_tracking', { …(3) } ]
🔇 Additional comments (7)
src/api/set/set.test.ts (1)
2-2: Import syntax updated correctly.The import statement has been appropriately updated from a default import to a named import, aligning with the corresponding changes in the implementation file.
src/api/time/time.ts (1)
4-4: Export declaration correctly updated.The export declaration has been properly changed from a default export to a named export, which improves consistency and makes imports more explicit.
src/api/set/set.ts (1)
6-6: Export declaration correctly updated.The export declaration has been properly changed from a default export to a named export, which improves consistency and makes imports more explicit.
src/api/time/time.test.ts (1)
2-2: Import syntax updated correctly.The import statement has been appropriately updated from a default import to a named import, aligning with the corresponding changes in the implementation file.
src/api/pageview/pageview.test.ts (1)
101-121: LGTM! Good test for URL normalization.This test correctly verifies that paths with and without trailing slashes are normalized to the same format, ensuring consistent tracking.
src/api/pageview/pageview.ts (2)
2-2: Good addition of the set import.Properly importing the
setfunction needed for UTM parameter handling.
48-50: Good implementation of URL normalization.This correctly removes trailing slashes from non-root paths, ensuring consistent URL tracking.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/api/pageview/pageview.ts (1)
12-20: Filter out only UTM parameters to avoid unintentional extraction.Currently, the code removes the "utm_" prefix but does not exclude non-UTM parameters. If you intend to extract only those parameters starting with
"utm_", you could filter them:function extractUtmParams(url: URL): Record<string, string> { const params: Record<string, string> = {}; - url.searchParams.forEach((value, key) => { - params[key.replace(UTM_PREFIX, "")] = value; + [...url.searchParams.entries()].forEach(([key, value]) => { + if (key.startsWith(UTM_PREFIX)) { + params[key.replace(UTM_PREFIX, "")] = value; + } }); return params; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/api/pageview/pageview.test.ts(1 hunks)src/api/pageview/pageview.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/api/pageview/pageview.test.ts
🔇 Additional comments (2)
src/api/pageview/pageview.ts (2)
2-2: Import usage validated.This import is used to set the extracted UTM parameters as campaign data. No issues found at this line.
10-10: Naming convention is clear.Defining
UTM_PREFIXis straightforward and enhances clarity for prefix-based parameter manipulation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package.json (1)
19-19: Potential Impact: Type-check Removal in Build ScriptBy changing the "build" script from
"tsc --noEmit && vite build"to"vite build", the project no longer performs a TypeScript type-check during the build phase. This might speed up the build process, but it also means that type errors could go unnoticed if not caught during continuous integration or via a separate type-checking process. Please ensure that you have an alternative mechanism (e.g., a dedicated CI step or a separate npm script like"typecheck": "tsc --noEmit") to catch type errors.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/utils.ts (2)
95-115: Add error handling for invalid URLsThe
useUtmParamsfunction creates a new URL object which will throw if the input is not a valid URL. Consider adding error handling to manage invalid URLs gracefully.export function useUtmParams(pageviewUrl: string): UseUtmParams { - const url = new URL(pageviewUrl); + try { + const url = new URL(pageviewUrl); + const utmParams: Record<string, string> = {}; + const params: string[] = []; + + url.searchParams.forEach((value, key) => { + if (key.includes(UTM_PREFIX)) { + utmParams[key.replace(UTM_PREFIX, "")] = value; + params.push(key); + } + }); + + for (const utmParam of params) { + url.searchParams.delete(utmParam); + } + + return { + utmParams, + cleanUrl: url.toString(), + }; + } catch (error) { + // Return original URL if invalid + return { + utmParams: {}, + cleanUrl: pageviewUrl, + }; + } }
117-122: Ensure URL normalization handles all edge casesThe
getPathWithBasefunction handles common edge cases like trailing and leading slashes, but may not handle all URL formatting scenarios. Consider usingURLconstructor or path libraries for more robust path handling, especially for complex URL structures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/api/pageview/pageview.test.ts(1 hunks)src/api/pageview/pageview.ts(2 hunks)src/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/api/pageview/pageview.test.ts
- src/api/pageview/pageview.ts
🔇 Additional comments (3)
src/utils.ts (3)
83-86: Type definition looks goodThe
UseUtmParamstype definition clearly defines the structure for the return value of theuseUtmParamsfunction, containing both the extracted UTM parameters and the cleaned URL.
88-88: Good use of constant for UTM prefixUsing a constant for the UTM prefix instead of hardcoding "utm_" throughout the code improves maintainability and consistency.
90-93: Simple and efficient UTM detectionThe
hasUtmParamsfunction uses a regular expression to efficiently detect the presence of UTM parameters in a URL. The implementation is concise and effective.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/utils.ts (3)
90-110: Consider adding URL validation for error handling.The function correctly extracts UTM parameters and cleans the URL, but it lacks input validation. If an invalid URL is passed, the
new URL(url)constructor will throw an error.export function useUtmParams(url: string): UseUtmParams { + try { const urlObject = new URL(url); const utmParams: Record<string, string> = {}; const params: string[] = []; urlObject.searchParams.forEach((value, key) => { if (key.includes(UTM_PREFIX)) { utmParams[key.replace(UTM_PREFIX, "")] = value; params.push(key); } }); for (const utmParam of params) { urlObject.searchParams.delete(utmParam); } return { utmParams, cleanUrl: urlObject.toString(), }; + } catch (error) { + // Return original URL if invalid + return { utmParams: {}, cleanUrl: url }; + } }
112-115: RegEx pattern might match partial parameters.The current regex pattern might match URLs where the UTM parameter is part of another parameter name (e.g.,
custom_utm_source). Consider using word boundaries or a more specific pattern.export function hasUtmParams(url: string): boolean { - const utmRegex = new RegExp(`[?&]${UTM_PREFIX}`); + const utmRegex = new RegExp(`[?&](${UTM_PREFIX}[^=&]*=)`); return !!url.match(utmRegex); }
117-122: Path concatenation looks good, but could handle more edge cases.The function properly handles trailing and leading slashes, but doesn't address URLs with query parameters or fragments.
Consider using the URL API for more robust URL handling:
export function getPathWithBase(path: string, base: string): string { - const normalizedBase = base.endsWith("/") ? base : `${base}/`; - const normalizedPath = path.startsWith("/") ? path.substring(1) : path; - - return `${normalizedBase}${normalizedPath}`; + try { + // Handle relative paths correctly + const baseUrl = new URL(base.endsWith("/") ? base : `${base}/`, window.location.href); + const fullPath = new URL(path, baseUrl); + return fullPath.toString(); + } catch (error) { + // Fallback to simple concatenation if URL parsing fails + const normalizedBase = base.endsWith("/") ? base : `${base}/`; + const normalizedPath = path.startsWith("/") ? path.substring(1) : path; + return `${normalizedBase}${normalizedPath}`; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/utils.ts(1 hunks)
🔇 Additional comments (2)
src/utils.ts (2)
83-83: Good use of a constant for UTM prefix.Using a constant for the UTM prefix ensures consistency across the codebase and avoids magic strings.
85-89: Clean type definition for UseUtmParams.The type definition is clear and well-structured, making the return value of the
useUtmParamsfunction easy to understand.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/utils.ts (1)
112-115: Improve UTM parameter detection regex.The regex in
hasUtmParamsshould be more specific to avoid false positives.export function hasUtmParams(url: string): boolean { - const utmRegex = new RegExp(`[?&]${UTM_PREFIX}`); + // Only match utm_ parameters in the query string, not in fragments + const utmRegex = new RegExp(`[?&]${UTM_PREFIX}[^&#=]*=`); return !!url.match(utmRegex); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/api/pageview/pageview.ts(3 hunks)src/utils.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/api/pageview/pageview.ts (2)
src/core/settings/types.ts (1)
Route(11-11)src/utils.ts (2)
hasUtmParams(112-115)useUtmParams(90-110)
🔇 Additional comments (7)
src/api/pageview/pageview.ts (4)
2-2: Appropriate use of the new import.The import of the
setfunction is correctly added to support the new UTM tracking functionality.
5-5: Import statement is properly updated.The import statement correctly includes the new utility functions
hasUtmParamsanduseUtmParamswhich are essential for the UTM parameter detection and handling.
11-11: Settings retrieval properly updated.The destructuring of
getSettings()correctly includes the newuseUtmTrackingoption which controls the UTM parameter tracking feature.
24-24: Good defensive programming practice.Converting
params.nameto a string ensures type safety for the page_title property.src/utils.ts (3)
83-83: Good use of constants for maintainability.Using a constant for the UTM prefix improves code maintainability and reduces the risk of inconsistencies.
85-88: Well-defined return type.The
UseUtmParamstype clearly defines the structure for the return value ofuseUtmParams, enhancing type safety and documentation.
117-122: Well-implemented path concatenation.The
getPathWithBasefunction correctly normalizes both the base and path components to ensure proper URL formation without duplicated or missing slashes.
src/api/pageview/pageview.ts
Outdated
| if (useUtmTracking && hasUtmParams(template.page_location)) { | ||
| const { utmParams, cleanUrl } = useUtmParams(template.page_location); | ||
|
|
||
| template.page_location = cleanUrl; | ||
| window.location.href = cleanUrl; | ||
|
|
||
| set("campaign", utmParams); | ||
| } |
There was a problem hiding this comment.
Reconsider modifying window.location.href.
The UTM tracking logic works correctly to extract parameters and clean the URL, but modifying window.location.href will cause a page reload, disrupting the user experience. Consider removing this line or making it optional.
if (useUtmTracking && hasUtmParams(template.page_location)) {
const { utmParams, cleanUrl } = useUtmParams(template.page_location);
template.page_location = cleanUrl;
- window.location.href = cleanUrl;
set("campaign", utmParams);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (useUtmTracking && hasUtmParams(template.page_location)) { | |
| const { utmParams, cleanUrl } = useUtmParams(template.page_location); | |
| template.page_location = cleanUrl; | |
| window.location.href = cleanUrl; | |
| set("campaign", utmParams); | |
| } | |
| if (useUtmTracking && hasUtmParams(template.page_location)) { | |
| const { utmParams, cleanUrl } = useUtmParams(template.page_location); | |
| template.page_location = cleanUrl; | |
| set("campaign", utmParams); | |
| } |
| export function useUtmParams(url: string): UseUtmParams { | ||
| const urlObject = new URL(url); | ||
| const utmParams: Record<string, string> = {}; | ||
| const params: string[] = []; | ||
|
|
||
| urlObject.searchParams.forEach((value, key) => { | ||
| if (key.includes(UTM_PREFIX)) { | ||
| utmParams[key.replace(UTM_PREFIX, "").replace("campaign", "id")] = value; | ||
| params.push(key); | ||
| } | ||
| }); | ||
|
|
||
| for (const utmParam of params) { | ||
| urlObject.searchParams.delete(utmParam); | ||
| } | ||
|
|
||
| return { | ||
| utmParams, | ||
| cleanUrl: urlObject.toString(), | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and document the campaign->id transformation.
The useUtmParams function is well structured but has two issues:
- It lacks error handling for invalid URLs
- The transformation of "campaign" to "id" in line 97 is not well documented and might be confusing
export function useUtmParams(url: string): UseUtmParams {
- const urlObject = new URL(url);
- const utmParams: Record<string, string> = {};
- const params: string[] = [];
+ try {
+ const urlObject = new URL(url);
+ const utmParams: Record<string, string> = {};
+ const params: string[] = [];
- urlObject.searchParams.forEach((value, key) => {
- if (key.includes(UTM_PREFIX)) {
- utmParams[key.replace(UTM_PREFIX, "").replace("campaign", "id")] = value;
- params.push(key);
- }
- });
+ urlObject.searchParams.forEach((value, key) => {
+ if (key.includes(UTM_PREFIX)) {
+ // Replace "campaign" with "id" to match Google Analytics campaign parameter naming
+ utmParams[key.replace(UTM_PREFIX, "").replace("campaign", "id")] = value;
+ params.push(key);
+ }
+ });
- for (const utmParam of params) {
- urlObject.searchParams.delete(utmParam);
- }
+ for (const utmParam of params) {
+ urlObject.searchParams.delete(utmParam);
+ }
- return {
- utmParams,
- cleanUrl: urlObject.toString(),
- };
+ return {
+ utmParams,
+ cleanUrl: urlObject.toString(),
+ };
+ } catch (error) {
+ console.error("Invalid URL:", url, error);
+ return {
+ utmParams: {},
+ cleanUrl: url,
+ };
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function useUtmParams(url: string): UseUtmParams { | |
| const urlObject = new URL(url); | |
| const utmParams: Record<string, string> = {}; | |
| const params: string[] = []; | |
| urlObject.searchParams.forEach((value, key) => { | |
| if (key.includes(UTM_PREFIX)) { | |
| utmParams[key.replace(UTM_PREFIX, "").replace("campaign", "id")] = value; | |
| params.push(key); | |
| } | |
| }); | |
| for (const utmParam of params) { | |
| urlObject.searchParams.delete(utmParam); | |
| } | |
| return { | |
| utmParams, | |
| cleanUrl: urlObject.toString(), | |
| }; | |
| } | |
| export function useUtmParams(url: string): UseUtmParams { | |
| try { | |
| const urlObject = new URL(url); | |
| const utmParams: Record<string, string> = {}; | |
| const params: string[] = []; | |
| urlObject.searchParams.forEach((value, key) => { | |
| if (key.includes(UTM_PREFIX)) { | |
| // Replace "campaign" with "id" to match Google Analytics campaign parameter naming | |
| utmParams[key.replace(UTM_PREFIX, "").replace("campaign", "id")] = value; | |
| params.push(key); | |
| } | |
| }); | |
| for (const utmParam of params) { | |
| urlObject.searchParams.delete(utmParam); | |
| } | |
| return { | |
| utmParams, | |
| cleanUrl: urlObject.toString(), | |
| }; | |
| } catch (error) { | |
| console.error("Invalid URL:", url, error); | |
| return { | |
| utmParams: {}, | |
| cleanUrl: url, | |
| }; | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/types/settings.ts (1)
208-213: Add@default false(and/or an explicit default) for the newuseUtmTrackingflagAll other boolean flags in
Settingsdocument their default behaviour.
For consistency and to avoid ambiguity for integrators, please state that the flag isfalseby default (and, ideally, make the runtime default explicit incore/settings.ts)./** * When enabled, automatically captures UTM parameters from URLs * and sends them as separate tracking events. + * @default false */ useUtmTracking?: boolean;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/api/pageview.ts(3 hunks)src/tests/api/pageview.test.ts(1 hunks)src/types/settings.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/api/pageview.ts (5)
src/types/gtag.ts (1)
GtagConfigParams(69-78)src/types/settings.ts (1)
Route(11-11)src/core/settings.ts (1)
getSettings(17-19)src/utils.ts (2)
hasUtmParams(112-115)useUtmParams(90-110)src/api/set.ts (1)
set(6-8)
🔇 Additional comments (1)
src/api/pageview.ts (1)
46-53: Possible mismatch between extracted UTM keys and test expectations
useUtmParams()currently convertsutm_campaign→id, yet the tests expect the keycampaign.
If the GA4 “campaign” field is intended, keep the original key; otherwise the new tests will fail.- utmParams[key.replace(UTM_PREFIX, "").replace("campaign", "id")] = value; + utmParams[key.replace(UTM_PREFIX, "")] = value;Please align either the utility or the tests before merging.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/tests/api/pageview.test.ts (1)
124-202: Consider adding tests for edge casesThe current tests cover the main functionality well, but consider adding tests for edge cases:
- When only some UTM parameters are present
- When
useUtmTrackingis enabled but no UTM parameters exist in the URLThese would enhance the robustness of your test suite.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
package.json(1 hunks)src/api/pageview.ts(3 hunks)src/tests/api/pageview.test.ts(2 hunks)src/utils.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/utils.ts
- src/api/pageview.ts
🔇 Additional comments (5)
src/tests/api/pageview.test.ts (5)
22-22: Added router replace spy for monitoring UTM cleaning operationsThis spy is effectively used in the new test at lines 178-202 to verify that UTM parameters are properly removed from the router query.
124-141: LGTM: Baseline test for UTM parameters behaviorThis test properly verifies the default behavior where UTM parameters are preserved in the page_location URL when UTM tracking is not enabled.
143-169: Good implementation of the custom UTM tracking testThis test correctly verifies that:
- UTM parameters are extracted and sent as a separate "campaign" object with the expected mapping
- The UTM parameters are removed from the page_location in the subsequent page_view event
- Other query parameters remain intact
The test meets the requirements of the PR objective to enable manual setting of UTM tags.
154-158: Fixed assertion to match implementationThis correctly uses
idinstead ofcampaignfor the UTM campaign parameter, resolving the issue identified in a previous review.
178-202: Good test for router query cleaning behaviorThis test effectively verifies that when both
pageTrackeranduseUtmTrackingare enabled, the router's query is properly updated to remove UTM parameters while preserving other parameters.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/utils.ts (1)
108-133:⚠️ Potential issueAdd error handling for invalid URLs.
The function is well-structured but lacks error handling for invalid URLs, which could cause runtime errors. The campaign->id transformation is now properly documented, which addresses part of the previous feedback.
export function useUtmParams(url: string): UseUtmParams { - const urlObject = new URL(url); - const utmParams: Record<string, string> = {}; - const params: string[] = []; - const cleanQueryParams: Record<string, string> = {}; + try { + const urlObject = new URL(url); + const utmParams: Record<string, string> = {}; + const params: string[] = []; + const cleanQueryParams: Record<string, string> = {}; - urlObject.searchParams.forEach((value, key) => { - if (key.includes(UTM_PREFIX)) { - // Replace "campaign" with "id" to match Google Analytics campaign parameter naming - utmParams[key.replace(UTM_PREFIX, "").replace("campaign", "id")] = value; - params.push(key); - } else { - cleanQueryParams[key] = value; - } - }); + urlObject.searchParams.forEach((value, key) => { + if (key.includes(UTM_PREFIX)) { + // Replace "campaign" with "id" to match Google Analytics campaign parameter naming + utmParams[key.replace(UTM_PREFIX, "").replace("campaign", "id")] = value; + params.push(key); + } else { + cleanQueryParams[key] = value; + } + }); - for (const utmParam of params) { - urlObject.searchParams.delete(utmParam); - } + for (const utmParam of params) { + urlObject.searchParams.delete(utmParam); + } - return { - utmParams, - cleanQueryParams, - cleanUrl: urlObject.toString(), - }; + return { + utmParams, + cleanQueryParams, + cleanUrl: urlObject.toString(), + }; + } catch (error) { + console.error("Invalid URL:", url, error); + return { + utmParams: {}, + cleanQueryParams: {}, + cleanUrl: url, + }; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/api/pageview.ts(3 hunks)src/tests/api/pageview.test.ts(3 hunks)src/utils.test.ts(1 hunks)src/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/tests/api/pageview.test.ts
- src/api/pageview.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/utils.ts (1)
src/api/query.ts (1)
QueryParams(5-5)
🔇 Additional comments (4)
src/utils.ts (4)
83-97: Clean implementation of URL query parameter replacement.This new utility function correctly handles replacing query parameters without triggering a page reload, with appropriate server-side check. It clears existing search parameters and sets new ones from the provided object.
99-106: Well-structured type definitions for UTM parameter handling.The UTM_PREFIX constant and UseUtmParams type clearly define the structure needed for UTM parameter handling, making the code more maintainable and type-safe.
135-138: Efficient implementation of UTM parameter detection.The regex-based approach is a performant way to check for UTM parameters in a URL without needing to create a URL object and iterate through all parameters.
140-145: Robust path concatenation handling.This utility correctly handles the edge cases of trailing and leading slashes when combining base URLs with paths, preventing common issues like double slashes or missing slashes.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/tests/utils.test.ts (1)
77-138: Well-structured and comprehensive test suite for URL query parameter replacement.The test suite for
urlQueryReplaceis well-implemented with proper setup and teardown of mocks forwindow.locationandwindow.history.replaceState. The tests cover key scenarios including replacing parameters, clearing all parameters, and handling empty values.Consider adding tests for the following edge cases:
- URL with special characters or URL-encoded values
- URL with hash fragments (e.g.,
#section)- URL with multiple occurrences of the same parameter
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/tests/index.test.ts(1 hunks)src/tests/utils.test.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/tests/index.test.ts
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/api/pageview.ts (1)
51-61: Well-implemented UTM parameter handlingThe implementation correctly:
- Detects presence of UTM parameters in the URL
- Extracts and transforms UTM parameters to campaign parameters
- Updates the page_location with a clean URL
- Updates the browser's URL query without reloading the page
- Sets the campaign data using the extracted UTM parameters
Consider adding error handling for potential URL parsing failures in case the page_location is invalid.
if (hasUtmParams(template.page_location)) { - const { utmParams, cleanUrl, cleanQueryParams } = useUtmParams( - template.page_location, - ); + try { + const { utmParams, cleanUrl, cleanQueryParams } = useUtmParams( + template.page_location, + ); + + template.page_location = cleanUrl; + + urlQueryReplace(cleanQueryParams); + + set("campaign", utmParams); + } catch (error) { + // Log error but continue with pageview + console.error("Failed to process UTM parameters:", error); + } - - template.page_location = cleanUrl; - - urlQueryReplace(cleanQueryParams); - - set("campaign", utmParams); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/api/pageview.ts(3 hunks)src/tests/api/pageview.test.ts(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/api/pageview.ts (2)
src/utils.ts (3)
hasUtmParams(135-138)useUtmParams(108-133)urlQueryReplace(83-97)src/api/set.ts (1)
set(6-8)
🔇 Additional comments (6)
src/tests/api/pageview.test.ts (3)
15-15: Good test setup for URL manipulation validationSetting up the spy on
urlQueryReplaceis a good approach to verify that the function is called correctly with the expected parameters when UTM parameters are present.
126-148: Well-structured test for UTM parameter extraction and URL cleaningThis test correctly validates two key aspects of the UTM handling functionality:
- That UTM parameters are extracted and set as campaign data
- That the page_location is cleaned from UTM parameters while preserving other query parameters
The test uses proper assertions and accurate expectations based on the implementation in the utils module.
157-174: Comprehensive test for router query UTM parameter cleanupThis test thoroughly verifies that when using the router with UTM parameters in the query, the
urlQueryReplaceutility is called with a cleaned query object that preserves non-UTM parameters. The test structure is clear and focused on a single responsibility.src/api/pageview.ts (3)
6-11: Good modularization of utility functionsImporting these utility functions from a shared module improves code organization and reusability. The functions have clear, descriptive names that indicate their purpose.
30-30: Safer type handling with explicit toString()Using explicit
toString()method instead of type assertion is a safer approach that avoids potential type errors at runtime.
51-61:✅ Verification successful
Verify URL handling when pageTracker is enabled
When pageTracker is enabled, there could be potential duplication in URL handling since the code always processes
template.page_locationfor UTM parameters, but there's a separate check for router query cleanup in the test. Ensure that this doesn't lead to double cleanup of UTM parameters.
🏁 Script executed:
#!/bin/bash # Check how pageview.ts and router integration work together # Search for places where urlQueryReplace is called rg -A 3 -B 3 "urlQueryReplace" --type tsLength of output: 2934
🏁 Script executed:
#!/bin/bash # Search for references to `pageTracker` in the codebase rg -R "pageTracker" --type tsLength of output: 434
🏁 Script executed:
#!/bin/bash # Corrected search for `pageTracker` references in the TypeScript codebase rg "pageTracker" --type ts -nLength of output: 2915
No duplicate URL cleanup detected
I’ve verified that
urlQueryReplaceis only called once insrc/api/pageview.tswhen processing UTM parameters. There are no additional calls in any pageTracker hooks or router integrations:
- Only occurrence of
urlQueryReplacein the runtime code is inside the UTM‐handling block inpageview.- No other modules (including core tracking or add-router-tracking) invoke
urlQueryReplace.No changes are needed here.
# [3.5.0](v3.4.0...v3.5.0) (2025-05-04) ### Features * **pageview:** extract and track utm parameters ([#618](#618)) ([b147fb9](b147fb9))
|
🎉 This PR is included in version 3.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
sets utm tags manually when using pageview
closes #517
closes #351
closes #405