Skip to content
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

feat(experiments): Apply no-code experiments to the webpage. #1409

Merged
merged 20 commits into from
Sep 13, 2024

Conversation

Phanatic
Copy link
Contributor

Changes

This PR introduces a new extension to posthog-js called web-experiments which allows posthog to apply no-code experiments to elements on a web page. This PR needs PostHog/posthog#24872 to merge so it can function.

To enable this feature, developers have to opt-in by setting disable_web_experiments to false in posthog.init while in Beta.

There are multiple ways to target an experiment within the browser here :

  1. Current URL can be matched with exact, contains, not_contains, regex, not_regex
  2. UTM targeting works by parsing the Current URL and evaluating conditions on individual variants.

Checklist

  • Tests for new code (see advice on the tests we use)
  • Accounted for the impact of any changes across different browsers
  • Accounted for backwards compatibility of any changes (no breaking changes in posthog-js!)

Coming up in following PRs

  • Support for dynamic elements on a webpage, via MutationObserver
  • Anti-flicker snippet support to prevent flickering of the webpage when applying web experiments.

Copy link

vercel bot commented Sep 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
posthog-js ✅ Ready (Inspect) Visit Preview Sep 13, 2024 6:59pm

@Phanatic Phanatic requested a review from a team September 10, 2024 15:54
@Phanatic Phanatic changed the title feat(experiments) : Apply no-code experiments to the webpage. feat(experiments): Apply no-code experiments to the webpage. Sep 10, 2024
Copy link

github-actions bot commented Sep 10, 2024

Size Change: +19.8 kB (+1.67%)

Total Size: 1.21 MB

Filename Size Change
dist/array.full.js 346 kB +4.96 kB (+1.45%)
dist/array.js 162 kB +4.95 kB (+3.15%)
dist/main.js 163 kB +4.95 kB (+3.14%)
dist/module.js 162 kB +4.95 kB (+3.15%)
ℹ️ View Unchanged
Filename Size
dist/exception-autocapture.js 10.4 kB
dist/recorder-v2.js 110 kB
dist/recorder.js 111 kB
dist/surveys-preview.js 59.8 kB
dist/surveys.js 66 kB
dist/tracing-headers.js 8.26 kB
dist/web-vitals.js 10.3 kB

compressed-size-action

Copy link
Contributor

@dmarticus dmarticus left a comment

Choose a reason for hiding this comment

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

looks reasonable overall, nice work! A couple of questions about data modeling and one about sanitizing HTML content but the rest looks fine.

src/web-experiments-types.ts Outdated Show resolved Hide resolved
src/web-experiments-types.ts Outdated Show resolved Hide resolved
Comment on lines +226 to +244
transform.attributes.forEach((attribute) => {
switch (attribute.name) {
case 'text':
htmlElement.innerText = attribute.value
break

case 'html':
htmlElement.innerHTML = attribute.value
break

case 'cssClass':
htmlElement.className = attribute.value
break

default:
htmlElement.setAttribute(attribute.name, attribute.value)
}
})
}
Copy link
Contributor

Choose a reason for hiding this comment

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

obviously users won't try to do this, and it's possible we're making this impossible on the server side, but should we sanitize the contents of these values that we're setting onto the DOM?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea about sanitizing payload. We can do that on the server-side with nh like we do for survey HTML

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