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

refactor: Remove/deprecate/rename old options #1694

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

rafaeelaudibert
Copy link
Member

@rafaeelaudibert rafaeelaudibert commented Jan 27, 2025

Changes

This was extracted from #1681

We're doing some different things here:

  • save_marketing_params over store_google because we do much more than just handling Google params with this settings
  • Deprecate verbose in favor of debug
  • Retire inapp_protocol and inapp_link_new_window, we're not using them anywhere, but we'll keep in the type for backwards-compatibility
  • Completely remove __preview_send_client_session_params, I've checked and it's not being used by anyone in production

I'm making this a minor bump, and then the remaining changes in #1681 will be only docs, and will be a patch version

@rafaeelaudibert rafaeelaudibert added the bump minor Bump minor version when this PR gets merged label Jan 27, 2025
@rafaeelaudibert rafaeelaudibert requested review from robbie-c and a team January 27, 2025 16:28
Copy link

vercel bot commented Jan 27, 2025

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

Name Status Preview Updated (UTC)
posthog-js ✅ Ready (Inspect) Visit Preview Jan 27, 2025 5:41pm

Copy link

github-actions bot commented Jan 27, 2025

Size Change: -1.19 kB (-0.04%)

Total Size: 3.28 MB

Filename Size Change
dist/array.full.es5.js 266 kB -118 B (-0.04%)
dist/array.full.js 371 kB -118 B (-0.03%)
dist/array.full.no-external.js 369 kB -118 B (-0.03%)
dist/array.js 182 kB -119 B (-0.07%)
dist/array.no-external.js 180 kB -119 B (-0.07%)
dist/main.js 182 kB -119 B (-0.07%)
dist/module.full.js 371 kB -118 B (-0.03%)
dist/module.full.no-external.js 369 kB -118 B (-0.03%)
dist/module.js 182 kB -119 B (-0.07%)
dist/module.no-external.js 180 kB -119 B (-0.07%)
ℹ️ View Unchanged
Filename Size
dist/all-external-dependencies.js 209 kB
dist/customizations.full.js 13.8 kB
dist/dead-clicks-autocapture.js 14.4 kB
dist/exception-autocapture.js 9.48 kB
dist/external-scripts-loader.js 2.64 kB
dist/recorder-v2.js 117 kB
dist/recorder.js 117 kB
dist/surveys-preview.js 67.5 kB
dist/surveys.js 64.2 kB
dist/tracing-headers.js 1.76 kB
dist/web-vitals.js 10.4 kB

compressed-size-action

@@ -133,20 +132,20 @@ export const defaultConfig = (): PostHogConfig => ({
persistence: 'localStorage+cookie', // up to 1.92.0 this was 'cookie'. It's easy to migrate as 'localStorage+cookie' will migrate data from cookie storage
persistence_name: '',
loaded: __NOOP,
store_google: true,
save_marketing_params: true,
Copy link
Member

Choose a reason for hiding this comment

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

Naming:

  • I'd probably call these attribution_params if I was starting from scratch. They aren't just for marketing, and product engineers should also care about them, because customers from different channels will have different levels of intent to use/buy your product :)
  • custom_campaign_params is calling them campaign_params - so we could also match that naming for consistency

Copy link
Member Author

@rafaeelaudibert rafaeelaudibert Jan 27, 2025

Choose a reason for hiding this comment

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

💯%, let me tweak that

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I tweaked to attribution, but we should instead keep it consistent, let me tweak it again

Copy link
Member Author

Choose a reason for hiding this comment

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

Now, really done, great suggestion

This is extracted from #1681

We're doing some different things here:
- `save_marketing_params` over `store_google` because we do much more than just handling Google params with this settings
- Deprecate `verbose` in favor of `debug`
- Retire `inapp_protocol` and `inapp_link_new_window`, we're not using them anywhere, but we'll keep in the type for backwards-compatibility
- Completely remove `__preview_send_client_session_params`, I've checked and it's not being used by anyone in production
@rafaeelaudibert rafaeelaudibert merged commit ff69389 into main Jan 27, 2025
28 checks passed
@rafaeelaudibert rafaeelaudibert deleted the deprecate-and-remove-unused-config branch January 27, 2025 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump minor Bump minor version when this PR gets merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants