fix(tiktok-pixel): use TikTok's array snippet protocol#786
Merged
Conversation
clientInit built window.ttq with the Facebook fbq callable protocol
(ttq.callMethod / ttq.queue) and called the bogus ttq('init', id).
TikTok's events.js consumes the official array protocol: ttq is an
Array of [method, ...args] tuples with ttq.methods / setAndDefer and
per-pixel _i/_t/_o scaffolding.
clientInit now builds the array protocol. use() returns a callable
adapter so both proxy.ttq('page') (legacy) and proxy.ttq.page() keep
working, forwarding to the live window.ttq array.
Adds an e2e suite + fixture asserting the array shape and that an
explicitly-tracked event reaches /api/v2/pixel.
Resolves #785
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
commit: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR replaces the fbq-style callable ttq with TikTok's array-based snippet protocol: new Ttq types, a typed TtqArray global, createTtqAdapter and TTQ_METHODS dispatcher, and client scaffolding that initializes window.ttq as an array with methods, per-pixel _i/_t/_o, consent application, and queued page() calls. It also adds a Nuxt fixture, expanded unit/type checks, and a comprehensive E2E suite that verifies protocol shape, compatibility with both push and method forms, script URL params, and conditional delivery capture. Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![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 (2)
test/types/types.test-d.ts (1)
187-191: ⚡ Quick winAdd explicit
event_idcoverage forProxyTtq['track']method form.This block currently proves proxy callable + method non-
event_id, but not method-formevent_idoverload preservation.Suggested test addition
it('proxy.ttq preserves both call forms', () => { type ProxyTtq = ReturnType<typeof useScriptTikTokPixel>['proxy']['ttq'] expectTypeOf<ProxyTtq>().toBeCallableWith('track', 'Purchase', { value: 10 }, { event_id: 'abc' }) + expectTypeOf<ProxyTtq['track']>().toBeCallableWith('Purchase', { value: 10 }, { event_id: 'abc' }) expectTypeOf<ProxyTtq['track']>().toBeCallableWith('StartTrial') })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/types/types.test-d.ts` around lines 187 - 191, Add a type-level assertion that the method-form track on the ProxyTtq preserves the event_id overload: inside the same test block that defines ProxyTtq (from useScriptTikTokPixel), add an expectTypeOf<ProxyTtq['track']>().toBeCallableWith('StartTrial', /* event data or undefined */ undefined, { event_id: 'abc' }) (or equivalent call shape matching the track signature) so the method-form event_id overload is explicitly covered.test/e2e/tiktok-pixel.test.ts (1)
118-133: ⚡ Quick winAdd a real callable-adapter assertion or rename this test.
Lines 131–132 validate method +
pushforms onwindow.ttq, but not the legacy callable adapter path (proxy.ttq('track', ...)) mentioned in the test title/comments. This leaves the stated backward-compat path unverified.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/tiktok-pixel.test.ts` around lines 118 - 133, Test title and comments claim to verify the legacy callable adapter path (proxy.ttq('track', ...)) but the test only exercises ttq.track and ttq.push; update the test to either (A) actually exercise the callable adapter by invoking proxy.ttq('track', 'ViewContent', { content_id: 'callable-adapter' }) inside the page.evaluate block and assert window.ttq length/contents changed accordingly (use the existing ttq, before/after and examine the appended tuple), or (B) if you don't intend to test proxy.ttq, rename the test and adjust the comment to reflect that it only verifies method and push forms (update the it(...) title and surrounding comment). Ensure references to proxy.ttq, window.ttq, ttq.track and ttq.push are used so the correct code paths are covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/e2e/tiktok-pixel.test.ts`:
- Around line 58-65: The current test grabs a script src string via
page.evaluate and asserts substrings; instead parse the found src into a URL
object and assert url.hostname === 'analytics.tiktok.com' and url.pathname ===
'/i18n/pixel/events.js' (or use startsWith for the pathname if query parts
vary), while keeping existing checks for query params like 'sdkid=TEST_PIXEL_ID'
and 'lib=ttq'; update the assertion block around the page.evaluate result (the
src variable in test/e2e/tiktok-pixel.test.ts) to construct new URL(src) and
perform hostname/pathname assertions rather than substring matching.
---
Nitpick comments:
In `@test/e2e/tiktok-pixel.test.ts`:
- Around line 118-133: Test title and comments claim to verify the legacy
callable adapter path (proxy.ttq('track', ...)) but the test only exercises
ttq.track and ttq.push; update the test to either (A) actually exercise the
callable adapter by invoking proxy.ttq('track', 'ViewContent', { content_id:
'callable-adapter' }) inside the page.evaluate block and assert window.ttq
length/contents changed accordingly (use the existing ttq, before/after and
examine the appended tuple), or (B) if you don't intend to test proxy.ttq,
rename the test and adjust the comment to reflect that it only verifies method
and push forms (update the it(...) title and surrounding comment). Ensure
references to proxy.ttq, window.ttq, ttq.track and ttq.push are used so the
correct code paths are covered.
In `@test/types/types.test-d.ts`:
- Around line 187-191: Add a type-level assertion that the method-form track on
the ProxyTtq preserves the event_id overload: inside the same test block that
defines ProxyTtq (from useScriptTikTokPixel), add an
expectTypeOf<ProxyTtq['track']>().toBeCallableWith('StartTrial', /* event data
or undefined */ undefined, { event_id: 'abc' }) (or equivalent call shape
matching the track signature) so the method-form event_id overload is explicitly
covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 59376210-4816-432e-b4f2-2a95620971a7
📒 Files selected for processing (12)
package.jsonpackages/script/src/registry-types.jsonpackages/script/src/runtime/registry/tiktok-pixel.tstest/e2e/tiktok-pixel.test.tstest/fixtures/tiktok-pixel/app.vuetest/fixtures/tiktok-pixel/nuxt.config.tstest/fixtures/tiktok-pixel/package.jsontest/fixtures/tiktok-pixel/pages/index.vuetest/fixtures/tiktok-pixel/pages/tiktok.vuetest/fixtures/tiktok-pixel/tsconfig.jsontest/nuxt-runtime/consent-default.nuxt.test.tstest/types/types.test-d.ts
Comment on lines
+58
to
+65
| const src = await page.evaluate(() => | ||
| Array.from(document.querySelectorAll<HTMLScriptElement>('script[src]')) | ||
| .map(s => s.src) | ||
| .find(s => s.includes('analytics.tiktok.com')), | ||
| ) | ||
| expect(src).toMatch(/analytics\.tiktok\.com\/i18n\/pixel\/events\.js/) | ||
| expect(src).toMatch(/sdkid=TEST_PIXEL_ID/) | ||
| expect(src).toMatch(/lib=ttq/) |
There was a problem hiding this comment.
Strengthen URL assertions with parsed hostname/path checks.
Line 61 currently accepts any URL containing analytics.tiktok.com as a substring, which can pass with the wrong host. Parse the URL and assert hostname/pathname directly.
Proposed fix
const src = await page.evaluate(() =>
Array.from(document.querySelectorAll<HTMLScriptElement>('script[src]'))
.map(s => s.src)
- .find(s => s.includes('analytics.tiktok.com')),
+ .find(s => s.includes('/i18n/pixel/events.js')),
)
- expect(src).toMatch(/analytics\.tiktok\.com\/i18n\/pixel\/events\.js/)
- expect(src).toMatch(/sdkid=TEST_PIXEL_ID/)
- expect(src).toMatch(/lib=ttq/)
+ if (!src) {
+ throw new Error('TikTok events.js script not found')
+ }
+ const parsed = new URL(src)
+ expect(parsed.hostname).toBe('analytics.tiktok.com')
+ expect(parsed.pathname).toBe('/i18n/pixel/events.js')
+ expect(parsed.searchParams.get('sdkid')).toBe('TEST_PIXEL_ID')
+ expect(parsed.searchParams.get('lib')).toBe('ttq')🧰 Tools
🪛 GitHub Check: CodeQL
[failure] 61-61: Incomplete URL substring sanitization
'analytics.tiktok.com' can be anywhere in the URL, and arbitrary hosts may come before or after it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/e2e/tiktok-pixel.test.ts` around lines 58 - 65, The current test grabs a
script src string via page.evaluate and asserts substrings; instead parse the
found src into a URL object and assert url.hostname === 'analytics.tiktok.com'
and url.pathname === '/i18n/pixel/events.js' (or use startsWith for the pathname
if query parts vary), while keeping existing checks for query params like
'sdkid=TEST_PIXEL_ID' and 'lib=ttq'; update the assertion block around the
page.evaluate result (the src variable in test/e2e/tiktok-pixel.test.ts) to
construct new URL(src) and perform hostname/pathname assertions rather than
substring matching.
CodeQL js/incomplete-url-substring-sanitization: parse the URL and compare hostname instead of String.includes.
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
2 participants
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.
🔗 Linked issue
Resolves #785
❓ Type of change
📚 Description
useScriptTikTokPixel'sclientInitbuiltwindow.ttqwith the Facebookfbqcallable protocol (ttq.callMethod/ttq.queue) and called a bogusttq('init', id). TikTok'sevents.jsconsumes the official array protocol:ttqis anArrayof[method, ...args]tuples withttq.methods/setAndDeferand per-pixel_i/_t/_oscaffolding.clientInitnow builds the array protocol. To stay backwards compatible,use()returns a callable adapter so bothproxy.ttq('page')(legacy) andproxy.ttq.page()work, forwarding to the livewindow.ttqarray.Adds an e2e suite + fixture (
test/fixtures/tiktok-pixel) asserting the array shape (nocallMethod/queueleftovers,_i._uscaffolding present) and that an explicitly-tracked event is delivered to/api/v2/pixel.