-
Notifications
You must be signed in to change notification settings - Fork 5.2k
feat: privacy-preserving A/B split testing Matomo integration #15716
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
Conversation
- ✅ Removed `NEXT_PUBLIC_` prefix from all A/B testing env vars - ✅ Updated documentation to reflect new naming convention
- Add core AB testing library with TypeScript types - Implement weighted variant assignment and cookie persistence - Add server-side configuration system with environment variables - Support multi-variant testing with configurable weights - Include proper error handling and development logging
- Create ABTestWrapper for server-side variant rendering - Add ABTestTracker for Matomo integration and cookie management - Implement development debug panel with variant switching - Support fallback rendering when tests are disabled
- Move AB components to dedicated directory structure - Remove old AB testing files from utils and root components - Update import paths to use new organized structure - Update stablecoins page to use new AB testing components - Add translation keys for new AB test variants
- Fix cookie prefix synchronization between server and client - Prevent infinite re-render loops in server components - Improve debug panel state management with useTransition - Add proper loading states and error handling - Enhance debug panel UX with reset buttons and tooltips
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
@claude can you review what we have in this draft PR so far |
Expand to read:
Claude finished @minimalsm's task —— View job AB Testing PR ReviewI've completed my review of the AB testing implementation. This is a well-structured and thoughtful implementation with good separation of concerns. Here's my detailed feedback: Review Tasks:
Architecture & Design 👍Strengths:
Issues Found 🔍Critical Issue: Matomo IntegrationFile: const push = (args: unknown[]) => {
console.log(`[Matomo] push called with args:`, args)
} The Matomo Recommendation: Remove the mock and use the real Matomo push function. Cookie Setting InconsistencyFiles: In Recommendation: Standardize cookie setting approach and ensure error handling is consistent. Type Safety GapsFile: export type ABTestVariants = [Element, Element, ...Element[]] While this enforces minimum 2 variants at compile time, runtime errors could still occur if fewer elements are provided to Recommendation: Add runtime validation in Code Quality Assessment ✅Positive Aspects:
Minor Improvements:
Implementation Review 📋Page Integration (
|
refactor to use IS_PROD and IS_PREVIEW_DEPLOY variables
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.
early review :) looking good
rm unused comments, rm env vars from example, rm redundant props, simplify to not require variant names (index order), update docs
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.
Much better with the server side approach! nice job @wackerow
Maybe Im missing something but i don't see the panel in the preview deploy and not sure exactly how to test it.
Other than that, the code looks fine 👍🏼
{}, | ||
{ | ||
headers: { | ||
"Cache-Control": "s-max-age=300, stale-while-revalidate=600", |
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.
do we need cache control for the error response?
Description
ABTestWrapper
component to pass array of test components toexperiments
information, including weights/traffic split, and potential scheduling restrictions assigned within Matomo experiment.localstorage
). In these environments, no events sent to Matomo.Workflow summary
Preview link
https://deploy-preview-15716--ethereumorg.netlify.app/en/stablecoins/