fix: DR-6518 UTM persistence #7384
Open
+57
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tentative fix on docs utm persistence
Contributor
Dangerous URL checkNo absolute URLs to prisma.io/docs found. |
Contributor
WalkthroughThe PR removes unused BorderBox imports from two content MDX files and introduces UTM parameter tracking functionality. A new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
Contributor
Contributor
Redirect checkThis PR probably requires the following redirects to be added to static/_redirects:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/theme/NavbarItem/NavbarNavLink.tsx (1)
38-55: Consider using the URL API for more robust parsing.After fixing the hash fragment issue, you could further improve robustness by using the native URL API. This handles all edge cases automatically and is more maintainable.
Example refactor using URL API:
const appendUtmParams = (url: string): string => { if (!utmParams) { return url; } try { // URL API requires absolute URLs for parsing, so we use a base const urlObj = new URL(url, 'http://placeholder'); // Append each UTM parameter const params = new URLSearchParams(utmParams); params.forEach((value, key) => { urlObj.searchParams.append(key, value); }); // Return the pathname, search, and hash (without the placeholder origin) return `${urlObj.pathname}${urlObj.search}${urlObj.hash}`; } catch (e) { // Fallback to original URL if parsing fails return url; } };Note: This approach is more robust but requires that
utmParamsis in the formatparam1=value1¶m2=value2.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
content/300-accelerate/index.mdx(0 hunks)content/700-optimize/index.mdx(0 hunks)src/hooks/useUTMParams.ts(1 hunks)src/theme/NavbarItem/NavbarNavLink.tsx(3 hunks)
💤 Files with no reviewable changes (2)
- content/700-optimize/index.mdx
- content/300-accelerate/index.mdx
🧰 Additional context used
🧬 Code graph analysis (1)
src/theme/NavbarItem/NavbarNavLink.tsx (1)
src/hooks/useUTMParams.ts (1)
useUTMParams(3-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Check internal links
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
src/theme/NavbarItem/NavbarNavLink.tsx (3)
35-36: LGTM! Hook usage is correct.The UTM parameters are correctly retrieved from the custom hook.
79-102: LGTM! External/internal link handling is correct.The logic correctly differentiates between external and internal links, applying UTM parameters only to internal navigation. The use of
prependBaseUrlToHrefis handled appropriately.
104-119: LGTM! The 'to' prop handling is correct.UTM parameters are correctly appended to the
toUrl, and all navigation props are preserved appropriately.
Comment on lines
+3
to
+15
| export const useUTMParams = (): string => { | ||
| const [utmParams, setUTMParams] = useState(''); | ||
|
|
||
| useEffect(() => { | ||
| // Check if we're on the client side | ||
| if (typeof window !== 'undefined') { | ||
| const storedParams = sessionStorage.getItem('utm_params'); | ||
| setUTMParams(storedParams || ''); | ||
| } | ||
| }, []); | ||
|
|
||
| return utmParams; | ||
| }; |
Contributor
There was a problem hiding this comment.
Validate and sanitize stored UTM parameters.
The hook reads from sessionStorage without validation. If the stored value is malformed or contains unexpected characters, it could break URL construction downstream or introduce security risks.
Consider adding validation:
export const useUTMParams = (): string => {
const [utmParams, setUTMParams] = useState('');
useEffect(() => {
// Check if we're on the client side
if (typeof window !== 'undefined') {
const storedParams = sessionStorage.getItem('utm_params');
- setUTMParams(storedParams || '');
+ // Validate that stored params look like query parameters
+ if (storedParams && /^[a-zA-Z0-9_=&%-]+$/.test(storedParams)) {
+ setUTMParams(storedParams);
+ } else {
+ setUTMParams('');
+ }
}
}, []);
return 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.
Suggested change
| export const useUTMParams = (): string => { | |
| const [utmParams, setUTMParams] = useState(''); | |
| useEffect(() => { | |
| // Check if we're on the client side | |
| if (typeof window !== 'undefined') { | |
| const storedParams = sessionStorage.getItem('utm_params'); | |
| setUTMParams(storedParams || ''); | |
| } | |
| }, []); | |
| return utmParams; | |
| }; | |
| export const useUTMParams = (): string => { | |
| const [utmParams, setUTMParams] = useState(''); | |
| useEffect(() => { | |
| // Check if we're on the client side | |
| if (typeof window !== 'undefined') { | |
| const storedParams = sessionStorage.getItem('utm_params'); | |
| // Validate that stored params look like query parameters | |
| if (storedParams && /^[a-zA-Z0-9_=&%-]+$/.test(storedParams)) { | |
| setUTMParams(storedParams); | |
| } else { | |
| setUTMParams(''); | |
| } | |
| } | |
| }, []); | |
| return utmParams; | |
| }; |
🤖 Prompt for AI Agents
In src/hooks/useUTMParams.ts around lines 3 to 15 the hook reads raw utm_params
from sessionStorage without validation; update the effect to safely read in a
try/catch, reject non-string values, parse the stored value using
URLSearchParams, whitelist only known UTM keys (utm_source, utm_medium,
utm_campaign, utm_term, utm_content), strip or percent-encode any characters
outside a safe set, rebuild a sanitized query string (e.g. key=value&...) using
encodeURIComponent for keys and values, and only setUTMParams with that
sanitized string (fallback to empty string on parse errors or if no valid keys).
Comment on lines
+38
to
+55
| // Helper function to append UTM params to URL | ||
| const appendUtmParams = (url: string): string => { | ||
| if (!utmParams) { | ||
| console.log('NavbarNavLink: No UTM params for URL:', url); | ||
| return url; | ||
| } | ||
|
|
||
| const [baseUrl, existingQuery] = url.split('?'); | ||
| if (existingQuery) { | ||
| const result = `${baseUrl}?${existingQuery}&${utmParams}`; | ||
| console.log('NavbarNavLink: Adding UTM params to existing query:', url, '->', result); | ||
| return result; | ||
| } else { | ||
| const result = `${baseUrl}?${utmParams}`; | ||
| console.log('NavbarNavLink: Adding UTM params to URL:', url, '->', result); | ||
| return result; | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
Fix URL parsing to handle hash fragments and remove debug logs.
The current implementation has two issues:
- Console.log statements should be removed before merging to production.
- Hash fragments are not handled correctly. URLs like
/docs/guide#sectionwould become/docs/guide#section?utm_source=xwhich is invalid. Query parameters must come before the hash.
Apply this diff to fix both issues:
- // Helper function to append UTM params to URL
- const appendUtmParams = (url: string): string => {
- if (!utmParams) {
- console.log('NavbarNavLink: No UTM params for URL:', url);
- return url;
- }
-
- const [baseUrl, existingQuery] = url.split('?');
- if (existingQuery) {
- const result = `${baseUrl}?${existingQuery}&${utmParams}`;
- console.log('NavbarNavLink: Adding UTM params to existing query:', url, '->', result);
- return result;
- } else {
- const result = `${baseUrl}?${utmParams}`;
- console.log('NavbarNavLink: Adding UTM params to URL:', url, '->', result);
- return result;
- }
- };
+ // Helper function to append UTM params to URL
+ const appendUtmParams = (url: string): string => {
+ if (!utmParams) {
+ return url;
+ }
+
+ // Split URL into base, query, and hash
+ const hashIndex = url.indexOf('#');
+ const hash = hashIndex !== -1 ? url.slice(hashIndex) : '';
+ const urlWithoutHash = hashIndex !== -1 ? url.slice(0, hashIndex) : url;
+
+ const [baseUrl, existingQuery] = urlWithoutHash.split('?');
+ const queryString = existingQuery ? `${existingQuery}&${utmParams}` : utmParams;
+
+ return `${baseUrl}?${queryString}${hash}`;
+ };📝 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.
Suggested change
| // Helper function to append UTM params to URL | |
| const appendUtmParams = (url: string): string => { | |
| if (!utmParams) { | |
| console.log('NavbarNavLink: No UTM params for URL:', url); | |
| return url; | |
| } | |
| const [baseUrl, existingQuery] = url.split('?'); | |
| if (existingQuery) { | |
| const result = `${baseUrl}?${existingQuery}&${utmParams}`; | |
| console.log('NavbarNavLink: Adding UTM params to existing query:', url, '->', result); | |
| return result; | |
| } else { | |
| const result = `${baseUrl}?${utmParams}`; | |
| console.log('NavbarNavLink: Adding UTM params to URL:', url, '->', result); | |
| return result; | |
| } | |
| }; | |
| // Helper function to append UTM params to URL | |
| const appendUtmParams = (url: string): string => { | |
| if (!utmParams) { | |
| return url; | |
| } | |
| // Split URL into base, query, and hash | |
| const hashIndex = url.indexOf('#'); | |
| const hash = hashIndex !== -1 ? url.slice(hashIndex) : ''; | |
| const urlWithoutHash = hashIndex !== -1 ? url.slice(0, hashIndex) : url; | |
| const [baseUrl, existingQuery] = urlWithoutHash.split('?'); | |
| const queryString = existingQuery ? `${existingQuery}&${utmParams}` : utmParams; | |
| return `${baseUrl}?${queryString}${hash}`; | |
| }; |
🤖 Prompt for AI Agents
In src/theme/NavbarItem/NavbarNavLink.tsx around lines 38 to 55, remove all
console.log debug statements and fix URL parsing so hash fragments are preserved
and query params are inserted before the fragment: if utmParams is falsy return
url unchanged; otherwise split the URL on '#' to separate fragment (keep
fragment if present), then split the pre-fragment part on '?' to detect existing
query string and append utmParams with the correct '&' or starting '?'
accordingly, finally reattach the '#' + fragment if it existed; ensure function
returns the correctly reconstructed string without any console logging.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
tentative fix on docs utm persistence
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.