Skip to content

Conversation

midhunadarvin
Copy link
Contributor

@midhunadarvin midhunadarvin commented Sep 24, 2025

Description

Gen 1 :

  • expose contentId on the builder class
  • update the trackConversion method to figure out contentId and variantionId if they are undefined from the SDK value and cookies.

Gen 2:

  • add track and trackConversion methods to the builder globals object.
  • add BuilderGlobalContext class to share context variables between components and helper functions in the gen2 sdks

Issue :
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.

  • Core (Gen1):
    • Expose Builder.contentId (reactively) and set it in React components on content load/inline render.
    • Enhance Builder.trackConversion to derive contentId from instance and variationId from test cookie; ignore variant when equal to content.
    • Add extensive unit tests for conversion parameter handling and edge cases.
  • SDKs (Gen2):
    • Add global context singleton (initializeGlobalBuilderContext) and inject setup script from Content to store apiKey, apiHost, and contentId.
    • Extend builder globals with track and trackConversion, using global context and getTestCookie.
    • Expose/init variant/helper scripts to window (incl. global context initializer) for A/B and personalization flows.
  • E2E/Specs:
    • New conversion tracking pages/specs for basic, variant (cookie-driven), symbol, and section scenarios.
    • Export test helpers; register new routes/content in specs.
  • Tooling/Docs:
    • Add watch:sdk script; document alternative Yarn link workflow in SDK develop docs.
  • Changeset: patch releases for affected @builder.io/* packages.

Written by Cursor Bugbot for commit 2ff9c7f. This will update automatically on new commits. Configure here.

Copy link

changeset-bot bot commented Sep 24, 2025

🦋 Changeset detected

Latest commit: 2ff9c7f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages
Name Type
@builder.io/react Patch
@builder.io/sdk Patch
@builder.io/sdk-angular Patch
@builder.io/sdk-react-nextjs Patch
@builder.io/sdk-qwik Patch
@builder.io/sdk-react Patch
@builder.io/sdk-react-native Patch
@builder.io/sdk-solid Patch
@builder.io/sdk-svelte Patch
@builder.io/sdk-vue Patch

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

Copy link

nx-cloud bot commented Sep 24, 2025

🤖 Nx Cloud AI Fix Eligible

An automatically generated fix could have helped fix failing tasks for this run, but Self-healing CI is disabled for this workspace. Visit workspace settings to enable it and get automatic fixes in future runs.

To disable these notifications, a workspace admin can disable them in workspace settings.


View your CI Pipeline Execution ↗ for commit 2ff9c7f

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

@midhunadarvin midhunadarvin changed the title feat: add contentId to builder class fix: conversion not being tracked properly with builder action Sep 25, 2025
Copy link
Contributor

@sidmohanty11 sidmohanty11 left a 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

Copy link

gitguardian bot commented Sep 29, 2025

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request
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
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. 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


🦉 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.

Comment on lines 45 to 61
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);
}
Copy link
Contributor

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?

Copy link
Contributor Author

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,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Zero Amounts Falsely Converted to Undefined

The trackConversion function incorrectly converts a 0 amount to undefined because amount || undefined treats 0 as falsy. This prevents valid zero-amount conversions from being tracked as intended.

Fix in Cursor Fix in Web

Comment on lines +32 to +50
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;
}
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants