-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: conversion not being tracked properly with builder action #4153
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 2ff9c7f The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Command | Status | Duration | Result |
---|---|---|---|
nx test @snippet/angular-17-ssr |
❌ Failed | 6m 23s | View ↗ |
☁️ Nx Cloud last updated this comment at 2025-10-01 05:38:09
UTC
contentId
to builder classThere 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.
looks good to me, can we add some tests for this?
there can be 3 e2e tests that we add:
- one page with the button and button calls track and we check amount,variationid,contentid etc
- one page with section and inside that a button calling track
- one page with symbol and calling track inside that
|
GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
---|---|---|---|---|---|
21273596 | Triggered | Generic High Entropy Secret | 9245f6d | packages/sdks-tests/src/specs/track-conversion.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
export function setTestsFromUrl() { | ||
if (typeof window === 'undefined') return; | ||
|
||
try { | ||
// Use native URL object to parse current page URL | ||
const params = parseUrlParams(window.location.href); | ||
|
||
// Look for parameters that start with 'builder.tests.' | ||
for (const [key, value] of params) { | ||
if (key.startsWith(`${testCookiePrefix}.`)) { | ||
const testKey = key.replace(`${testCookiePrefix}.`, ''); | ||
setTestCookie(testKey, value); | ||
} | ||
} | ||
} catch (e) { | ||
console.debug('Error parsing tests from URL', e); | ||
} |
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.
all of these functions are not used anymore right?
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.
yes, not used anymore. Thanks for catching. removed the unused functions
type: 'conversion', | ||
apiHost: builderContext?.apiHost, | ||
apiKey: builderContext?.apiKey || '', | ||
amount: amount || undefined, |
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.
export function initializeGlobalBuilderContext(): void { | ||
// Detect environment and get the appropriate global object | ||
const isServer = typeof window === 'undefined'; | ||
const globalObject = isServer | ||
? typeof globalThis !== 'undefined' | ||
? globalThis | ||
: (function () { | ||
try { | ||
return global; | ||
} catch (e) { | ||
return {}; | ||
} | ||
})() | ||
: window; | ||
|
||
if ((globalObject as any).GlobalBuilderContext) { | ||
// if already exists, don't re-initialize | ||
return; | ||
} |
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.
As discussed on the call, I recommend avoiding a new global context, as maintaining it would create unpredictable problems across SDKs.
- SSR frameworks could behave in unexpected ways (two different global contexts, also cookie isn't present on server)
- This will be an extra addition to the customer’s HTML as we're injecting inlined scripts
- It’s extra code that we have to maintain.
The simpler solution is to expose just track
and trackConversion
on the Gen2 SDK globals obj and update every evaluate
call to include apiKey and contentId.
Description
Gen 1 :
contentId
on the builder classtrackConversion
method to figure outcontentId
andvariantionId
if they are undefined from the SDK value and cookies.Gen 2:
BuilderGlobalContext
class to share context variables between components and helper functions in the gen2 sdksIssue :
https://www.loom.com/share/6bcecbfc1b284331a867f162223e4c20
Loom - Gen 1 fix
https://www.loom.com/share/3712e28c436b4d9eac9bd3e6a2a68325
Loom - Gen 2 fix
https://www.loom.com/share/fc3b894372d244d8b0c2a14e67ed93ec
Note
Ensure conversions capture contentId and variationId via instance/global context and cookies, adding tracking APIs, inlined context setup, and comprehensive tests.
Builder.contentId
(reactively) and set it in React components on content load/inline render.Builder.trackConversion
to derivecontentId
from instance andvariationId
from test cookie; ignore variant when equal to content.initializeGlobalBuilderContext
) and inject setup script fromContent
to storeapiKey
,apiHost
, andcontentId
.builder
globals withtrack
andtrackConversion
, using global context andgetTestCookie
.watch:sdk
script; document alternative Yarn link workflow in SDK develop docs.@builder.io/*
packages.Written by Cursor Bugbot for commit 2ff9c7f. This will update automatically on new commits. Configure here.