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.

Sign up for GitHub

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

Jump to bottom

feat: Added global state check for toolbar #1173

Merged
merged 3 commits into from
May 8, 2024
Merged

feat: Added global state check for toolbar #1173

merged 3 commits into from
May 8, 2024

Conversation

benjackwhite
Copy link
Collaborator

Changes

Fixes #1172

You can have multiple SDKs running in different configs. This can lead to some issues with the toolbar as both instances willt try to load it.

  • Save the loading state on the window so that mulitple SDKs reference the same thing
  • Improve the load logic to check for the actual toolbar ID to decide whether to load it or not.

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

@benjackwhite benjackwhite added the bump patch Bump patch version when this PR gets merged label May 8, 2024
@vercel Vercel
Copy link

vercel bot commented May 8, 2024 •
edited

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

Name Status Preview Updated (UTC)
posthog-js ✅ Ready (Inspect) Visit Preview May 8, 2024 10:32am

@benjackwhite benjackwhite changed the title feat: Added global state for toolbar feat: Added global state check for toolbar May 8, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented May 8, 2024 •
edited

Size Change: +1.41 kB (0%)

Total Size: 975 kB

Filename Size Change
dist/array.full.js 234 kB +355 B (0%)
dist/array.js 132 kB +352 B (0%)
dist/es.js 132 kB +352 B (0%)
dist/module.js 132 kB +352 B (0%)
ℹ️ View Unchanged
Filename Size
dist/exception-autocapture.js 12.2 kB
dist/recorder-v2.js 108 kB
dist/recorder.js 108 kB
dist/surveys-module-previews.js 57.2 kB
dist/surveys.js 58.8 kB

compressed-size-action

pauldambra
pauldambra approved these changes May 8, 2024
View reviewed changes
Copy link
Member

@pauldambra pauldambra left a comment •
edited

Choose a reason for hiding this comment

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

legibility nit otherwise :shipit:

(although the tests are unhappy 🙈 )

@benjackwhite benjackwhite merged commit 7e082fa into main May 8, 2024
12 checks passed
@benjackwhite benjackwhite deleted the fix/toolbar-multi-sdks branch May 8, 2024 11:17
@Bad3r Bad3r mentioned this pull request May 29, 2024
Open
@simlarsen simlarsen mentioned this pull request May 30, 2024
Merged
@ITSecOps-404 ITSecOps-404 mentioned this pull request May 31, 2024
Open
@Bad3r Bad3r mentioned this pull request May 31, 2024
Open
@simlarsen simlarsen mentioned this pull request Jun 1, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pauldambra pauldambra pauldambra approved these changes

Assignees
No one assigned
Labels
bump patch Bump patch version when this PR gets merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Toolbar loads incorrectly when using multiple posthog instances
2 participants
@benjackwhite @pauldambra