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: Load site functions via RemoteConfig #1580

Merged
merged 23 commits into from
Dec 9, 2024
Merged

Conversation

benjackwhite
Copy link
Collaborator

@benjackwhite benjackwhite commented Dec 5, 2024

Changes

Needs #1581
Needs PostHog/posthog#26671

  • Moves the site functions to only be loaded the new way
  • Changes the loading mentality so that the caller is responsible for initializing and passing things in (this is pretty important as we don't want the server side code to be calling non-top-level methods as they might change over time).

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!)

Copy link

vercel bot commented Dec 5, 2024

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

Name Status Preview Updated (UTC)
posthog-js ✅ Ready (Inspect) Visit Preview Dec 6, 2024 9:37am

@posthog-bot
Copy link
Collaborator

Hey @benjackwhite! 👋
This pull request seems to contain no description. Please add useful context, rationale, and/or any other information that will help make sense of this change now and in the distant Mars-based future.

@benjackwhite benjackwhite changed the base branch from main to feat/serverless-decide December 5, 2024 12:34
Copy link

github-actions bot commented Dec 5, 2024

Size Change: +11 kB (+0.35%)

Total Size: 3.21 MB

Filename Size Change
dist/array.full.es5.js 260 kB +1.27 kB (+0.49%)
dist/array.full.js 364 kB +1.08 kB (+0.3%)
dist/array.full.no-external.js 362 kB +1.08 kB (+0.3%)
dist/array.js 178 kB +1.08 kB (+0.61%)
dist/array.no-external.js 176 kB +1.08 kB (+0.62%)
dist/main.js 179 kB +1.08 kB (+0.61%)
dist/module.full.js 364 kB +1.08 kB (+0.3%)
dist/module.full.no-external.js 362 kB +1.08 kB (+0.3%)
dist/module.js 178 kB +1.08 kB (+0.61%)
dist/module.no-external.js 177 kB +1.08 kB (+0.62%)
ℹ️ View Unchanged
Filename Size
dist/all-external-dependencies.js 206 kB
dist/customizations.full.js 12.6 kB
dist/dead-clicks-autocapture.js 14.4 kB
dist/exception-autocapture.js 9.48 kB
dist/external-scripts-loader.js 2.48 kB
dist/recorder-v2.js 115 kB
dist/recorder.js 115 kB
dist/surveys-preview.js 57.6 kB
dist/surveys.js 63.3 kB
dist/tracing-headers.js 1.75 kB
dist/web-vitals.js 10.3 kB

compressed-size-action

Base automatically changed from feat/serverless-decide to main December 5, 2024 14:55
# Conflicts:
#	playground/nextjs/src/posthog.ts
#	src/__tests__/site-apps.ts
#	src/site-apps.ts
#	src/types.ts
#	src/utils/globals.ts
__preview_remote_config: true,
...configForConsent(),
})
// Help with debugging(window as any).posthog = posthog
// Help with debugging
;(window as any).posthog = posthog
Copy link
Collaborator Author

@benjackwhite benjackwhite Dec 6, 2024

Choose a reason for hiding this comment

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

Decided this should always be on as its super useful when testing around

src/types.ts Outdated Show resolved Hide resolved
# Conflicts:
#	src/__tests__/site-apps.ts
#	src/site-apps.ts
#	src/utils/logger.ts
Copy link
Collaborator

@mariusandra mariusandra 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. The only question I had was if legacy site apps still work, but reading the code more it seems like that's handled. So ✅

@benjackwhite
Copy link
Collaborator Author

Looks good. The only question I had was if legacy site apps still work, but reading the code more it seems like that's handled. So ✅

They should do. The actual path for loading them is basically identical to before. I'll test again for sure before merging thouh.

@benjackwhite benjackwhite added the bump patch Bump patch version when this PR gets merged label Dec 9, 2024
@benjackwhite benjackwhite merged commit b16c6c8 into main Dec 9, 2024
16 checks passed
@benjackwhite benjackwhite deleted the feat/site-apps-loading branch December 9, 2024 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump patch Bump patch version when this PR gets merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants