Add wildcard support to outbound-domains#63
Merged
steven-tey merged 2 commits intomainfrom Sep 4, 2025
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdded wildcard domain support for outbound link tracking by matching patterns like Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant P as Outbound Page
participant S as Outbound Script
participant M as isMatchingDomain
U->>P: GET /outbound?dub_id=...
P->>S: Init analytics with domains (incl. "*.wildcard.com")
S->>P: Scan anchors
loop per anchor
S->>M: isMatchingDomain(configDomain, anchor.href)
alt configDomain starts with "*."
Note right of M #eef3ff: New path — match by suffix\nhostname.endsWith(apexDomain)
M-->>S: true/false
else
Note right of M #fff7e6: Existing path — normalize and exact match
M-->>S: true/false
end
alt match == true
S->>P: Append dub_id to href
else
S-->>P: Leave href unchanged
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 1
🧹 Nitpick comments (4)
packages/script/src/extensions/outbound-domains.js (1)
26-27: Exact-match path is fine; consider normalizing case to be robust.URL.hostnames are lowercased, but config values may not be. Consider lowercasing in normalizeDomain for consistency.
Outside-range helper suggestion:
function normalizeDomain(domain) { return domain.toLowerCase().replace(/^www\./, '').replace(/\.$/, '').trim(); }apps/nextjs/app/layout.tsx (1)
28-29: Confirm requirement for apex matching with wildcard.If you pick Option B (exclude apex) in the script, add "wildcard.com" explicitly to outbound domains. If you pick Option A (include apex), current config is sufficient but implicitly relies on that behavior. Please confirm product intent.
apps/nextjs/tests/outbound-domains.spec.ts (2)
30-34: Prefer locators + attribute assertions to remove flakiness and extra waitsUse Playwright’s auto-waiting instead of manual element handles. This also adds a regex boundary to avoid false positives or duplicates.
- // Also verify wildcard domain functionality - const wildcardLink = await page.$('a[href*="api.wildcard.com"]'); - const wildcardHref = await wildcardLink?.getAttribute('href'); - expect(wildcardHref).toContain('dub_id=test-click-id'); + // Also verify wildcard domain functionality + await expect( + page.locator('a[href*="://api.wildcard.com"]'), + ).toHaveAttribute('href', /[?&]dub_id=test-click-id(&|$)/);
191-210: Simplify with locators and drop arbitrary timeoutSame benefits: auto-wait, clearer failures, and boundary-safe regex.
- await page.waitForTimeout(1000); - - // Test wildcard domain with www prefix - const wwwWildcardLink = await page.$('a[href*="www.api.wildcard.com"]'); - const wwwWildcardSubdomainLink = await page.$( - 'a[href*="www.admin.wildcard.com"]', - ); - - const wwwWildcardHref = await wwwWildcardLink?.getAttribute('href'); - const wwwWildcardSubdomainHref = - await wwwWildcardSubdomainLink?.getAttribute('href'); - - // www. prefix should be ignored, so these should still match wildcard pattern - expect(wwwWildcardHref).toContain('dub_id=test-click-id'); - expect(wwwWildcardSubdomainHref).toContain('dub_id=test-click-id'); + // Test wildcard domain with www prefix + const wwwApi = page.locator('a[href*="://www.api.wildcard.com"]'); + const wwwAdmin = page.locator('a[href*="://www.admin.wildcard.com"]'); + const hasDubId = /[?&]dub_id=test-click-id(&|$)/; + + // www. prefix should be ignored, so these should still match wildcard pattern + await expect(wwwApi).toHaveAttribute('href', hasDubId); + await expect(wwwAdmin).toHaveAttribute('href', hasDubId);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/nextjs/app/layout.tsx(1 hunks)apps/nextjs/app/outbound/page.tsx(1 hunks)apps/nextjs/tests/outbound-domains.spec.ts(2 hunks)packages/script/src/extensions/outbound-domains.js(1 hunks)
⏰ 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). (1)
- GitHub Check: Vade Review
🔇 Additional comments (2)
packages/script/src/extensions/outbound-domains.js (1)
21-25: Enforce dot-boundary in wildcard matching
The currentendsWith(normalizedDomain.slice(2))will match “notwildcard.com” or “evilwildcard.com.” Replace it with one of the following:Option A – include apex match:
- if (normalizedDomain.startsWith('*.')) { - return normalizedUrlHostname.endsWith(normalizedDomain.slice(2)); - } + if (normalizedDomain.startsWith('*.')) { + const suffix = normalizedDomain.slice(2); + return ( + normalizedUrlHostname === suffix || + normalizedUrlHostname.endsWith(`.${suffix}`) + ); + }Option B – strict “*.domain.com” semantics (exclude apex):
- if (normalizedDomain.startsWith('*.')) { - return normalizedUrlHostname.endsWith(normalizedDomain.slice(2)); - } + if (normalizedDomain.startsWith('*.')) { + const suffix = normalizedDomain.slice(2); + return normalizedUrlHostname.endsWith(`.${suffix}`); + }Which behavior is intended for
*.wildcard.com: should it match the apex “wildcard.com” (Option A) or only subdomains (Option B)?apps/nextjs/app/outbound/page.tsx (1)
19-26: Enforce dot-boundary matching for ‘*.wildcard.com’
Ensure the outbound-domain matcher treats ‘*.wildcard.com’ as matching only true subdomains (e.g.api.wildcard.com) and notnotwildcard.com. Verify and update the analytics wildcard logic to include the leading “.” boundary.
Comment on lines
+156
to
+189
|
|
||
| test('should handle wildcard domains correctly', async ({ page }) => { | ||
| await page.goto('/outbound?dub_id=test-click-id'); | ||
| await page.waitForFunction(() => window._dubAnalytics !== undefined); | ||
|
|
||
| await page.waitForTimeout(1000); | ||
|
|
||
| // Test wildcard domain matching - *.wildcard.com should match all subdomains | ||
| const wildcardLink = await page.$('a[href*="api.wildcard.com"]'); | ||
| const wildcardSubdomainLink = await page.$('a[href*="admin.wildcard.com"]'); | ||
| const wildcardNestedLink = await page.$( | ||
| 'a[href*="deep.nested.wildcard.com"]', | ||
| ); | ||
| const wildcardRootLink = await page.$('a[href*="wildcard.com"]'); | ||
|
|
||
| // Test non-wildcard domain that shouldn't match wildcard pattern | ||
| const nonWildcardLink = await page.$('a[href*="notwildcard.com"]'); | ||
|
|
||
| const wildcardHref = await wildcardLink?.getAttribute('href'); | ||
| const wildcardSubdomainHref = | ||
| await wildcardSubdomainLink?.getAttribute('href'); | ||
| const wildcardNestedHref = await wildcardNestedLink?.getAttribute('href'); | ||
| const wildcardRootHref = await wildcardRootLink?.getAttribute('href'); | ||
| const nonWildcardHref = await nonWildcardLink?.getAttribute('href'); | ||
|
|
||
| // All wildcard.com subdomains should have tracking | ||
| expect(wildcardHref).toContain('dub_id=test-click-id'); | ||
| expect(wildcardSubdomainHref).toContain('dub_id=test-click-id'); | ||
| expect(wildcardNestedHref).toContain('dub_id=test-click-id'); | ||
| expect(wildcardRootHref).toContain('dub_id=test-click-id'); | ||
|
|
||
| // Non-wildcard domain should not have tracking | ||
| expect(nonWildcardHref).not.toContain('dub_id=test-click-id'); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apex-domain selector is ambiguous; may match subdomains and cause flaky failures
'a[href*="wildcard.com"]' can resolve to any subdomain and even collide with non-targets depending on order. Also replace manual timeouts with auto-waits and assert using regex boundaries.
- await page.waitForTimeout(1000);
+ // Rely on auto-waiting expectations instead of arbitrary sleeps.
- // Test wildcard domain matching - *.wildcard.com should match all subdomains
- const wildcardLink = await page.$('a[href*="api.wildcard.com"]');
- const wildcardSubdomainLink = await page.$('a[href*="admin.wildcard.com"]');
- const wildcardNestedLink = await page.$(
- 'a[href*="deep.nested.wildcard.com"]',
- );
- const wildcardRootLink = await page.$('a[href*="wildcard.com"]');
+ // Test wildcard domain matching - *.wildcard.com should match all subdomains
+ const api = page.locator('a[href*="://api.wildcard.com"]');
+ const admin = page.locator('a[href*="://admin.wildcard.com"]');
+ const nested = page.locator('a[href*="://deep.nested.wildcard.com"]');
+ // Apex only; avoid matching subdomains
+ const apex = page
+ .locator('a[href="https://wildcard.com"], a[href="https://wildcard.com/"]')
+ .first();
- // Test non-wildcard domain that shouldn't match wildcard pattern
- const nonWildcardLink = await page.$('a[href*="notwildcard.com"]');
+ // Test non-wildcard domain that shouldn't match wildcard pattern
+ const nonWildcard = page.locator('a[href*="://notwildcard.com"]');
- const wildcardHref = await wildcardLink?.getAttribute('href');
- const wildcardSubdomainHref =
- await wildcardSubdomainLink?.getAttribute('href');
- const wildcardNestedHref = await wildcardNestedLink?.getAttribute('href');
- const wildcardRootHref = await wildcardRootLink?.getAttribute('href');
- const nonWildcardHref = await nonWildcardLink?.getAttribute('href');
+ const hasDubId = /[?&]dub_id=test-click-id(&|$)/;
- // All wildcard.com subdomains should have tracking
- expect(wildcardHref).toContain('dub_id=test-click-id');
- expect(wildcardSubdomainHref).toContain('dub_id=test-click-id');
- expect(wildcardNestedHref).toContain('dub_id=test-click-id');
- expect(wildcardRootHref).toContain('dub_id=test-click-id');
+ // All wildcard.com subdomains should have tracking
+ await expect(api).toHaveAttribute('href', hasDubId);
+ await expect(admin).toHaveAttribute('href', hasDubId);
+ await expect(nested).toHaveAttribute('href', hasDubId);
+ await expect(apex).toHaveAttribute('href', hasDubId);
- // Non-wildcard domain should not have tracking
- expect(nonWildcardHref).not.toContain('dub_id=test-click-id');
+ // Non-wildcard domain should not have tracking
+ await expect(nonWildcard).not.toHaveAttribute('href', hasDubId);📝 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
| test('should handle wildcard domains correctly', async ({ page }) => { | |
| await page.goto('/outbound?dub_id=test-click-id'); | |
| await page.waitForFunction(() => window._dubAnalytics !== undefined); | |
| await page.waitForTimeout(1000); | |
| // Test wildcard domain matching - *.wildcard.com should match all subdomains | |
| const wildcardLink = await page.$('a[href*="api.wildcard.com"]'); | |
| const wildcardSubdomainLink = await page.$('a[href*="admin.wildcard.com"]'); | |
| const wildcardNestedLink = await page.$( | |
| 'a[href*="deep.nested.wildcard.com"]', | |
| ); | |
| const wildcardRootLink = await page.$('a[href*="wildcard.com"]'); | |
| // Test non-wildcard domain that shouldn't match wildcard pattern | |
| const nonWildcardLink = await page.$('a[href*="notwildcard.com"]'); | |
| const wildcardHref = await wildcardLink?.getAttribute('href'); | |
| const wildcardSubdomainHref = | |
| await wildcardSubdomainLink?.getAttribute('href'); | |
| const wildcardNestedHref = await wildcardNestedLink?.getAttribute('href'); | |
| const wildcardRootHref = await wildcardRootLink?.getAttribute('href'); | |
| const nonWildcardHref = await nonWildcardLink?.getAttribute('href'); | |
| // All wildcard.com subdomains should have tracking | |
| expect(wildcardHref).toContain('dub_id=test-click-id'); | |
| expect(wildcardSubdomainHref).toContain('dub_id=test-click-id'); | |
| expect(wildcardNestedHref).toContain('dub_id=test-click-id'); | |
| expect(wildcardRootHref).toContain('dub_id=test-click-id'); | |
| // Non-wildcard domain should not have tracking | |
| expect(nonWildcardHref).not.toContain('dub_id=test-click-id'); | |
| }); | |
| test('should handle wildcard domains correctly', async ({ page }) => { | |
| await page.goto('/outbound?dub_id=test-click-id'); | |
| await page.waitForFunction(() => window._dubAnalytics !== undefined); | |
| // Rely on auto-waiting expectations instead of arbitrary sleeps. | |
| // Test wildcard domain matching - *.wildcard.com should match all subdomains | |
| const api = page.locator('a[href*="://api.wildcard.com"]'); | |
| const admin = page.locator('a[href*="://admin.wildcard.com"]'); | |
| const nested = page.locator('a[href*="://deep.nested.wildcard.com"]'); | |
| // Apex only; avoid matching subdomains | |
| const apex = page | |
| .locator('a[href="https://wildcard.com"], a[href="https://wildcard.com/"]') | |
| .first(); | |
| // Test non-wildcard domain that shouldn't match wildcard pattern | |
| const nonWildcard = page.locator('a[href*="://notwildcard.com"]'); | |
| const hasDubId = /[?&]dub_id=test-click-id(&|$)/; | |
| // All wildcard.com subdomains should have tracking | |
| await expect(api).toHaveAttribute('href', hasDubId); | |
| await expect(admin).toHaveAttribute('href', hasDubId); | |
| await expect(nested).toHaveAttribute('href', hasDubId); | |
| await expect(apex).toHaveAttribute('href', hasDubId); | |
| // Non-wildcard domain should not have tracking | |
| await expect(nonWildcard).not.toHaveAttribute('href', hasDubId); | |
| }); |
🤖 Prompt for AI Agents
In apps/nextjs/tests/outbound-domains.spec.ts around lines 156 to 189, the
selector 'a[href*="wildcard.com"]' is ambiguous and can match subdomains or
unintended links, and the test uses a manual waitForTimeout; change selectors to
verify link hostnames explicitly (parse each anchor href with new URL(...) and
assert hostname === 'wildcard.com' or hostname.endsWith('.wildcard.com') for
wildcard cases), replace the generic 'a[href*="wildcard.com"]' with precise
queries (e.g., locate anchors then filter by parsed hostname), ensure the
non-wildcard case checks hostname does not match '*.wildcard.com', replace
waitForTimeout with deterministic waits like waitForSelector or waitForFunction
that _dubAnalytics and the specific anchors are present, and change the
assertions to use regex boundary checks (e.g., match dub_id=test-click-id as a
whole token) to avoid false positives.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit
New Features
Tests