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: Toolbar loading from state faster #849

Merged
merged 2 commits into from
Oct 25, 2023
Merged

Conversation

benjackwhite
Copy link
Collaborator

Changes

Seems like a lot of web frameworks get in the way of us pulling the state from the location hash. Hacky fix for this is to try and get this info the moment the page loads, rather than after the sdk is initialised.

  • Adds that
  • Also adds support for base64 decoding the state data which allows us to eventually swap to b64 encoding which is even less likely to get messed up by web frameworks.

Checklist

@github-actions
Copy link

Size Change: +623 B (0%)

Total Size: 723 kB

Filename Size Change
dist/array.full.js 176 kB +167 B (0%)
dist/array.js 118 kB +152 B (0%)
dist/es.js 118 kB +152 B (0%)
dist/module.js 118 kB +152 B (0%)
ℹ️ View Unchanged
Filename Size
dist/recorder-v2.js 95.1 kB
dist/recorder.js 58.4 kB
dist/surveys.js 39.6 kB

compressed-size-action

import { PostHog } from '../posthog-core'
import { DecideResponse, ToolbarParams } from '../types'
import { POSTHOG_MANAGED_HOSTS } from './cloud'

// TRICKY: Many web frameworks will modify the route on load, potentially before posthog is initialized.
// To get ahead of this we grab it as soon as the posthog-js is parsed
const STATE_FROM_WINDOW = window.location
Copy link
Member

Choose a reason for hiding this comment

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

sneaky but interesting 😊

@@ -274,6 +274,14 @@ export const _formatDate = function (d: Date): string {
)
}

export const _try = function <T>(fn: () => T): T | undefined {
Copy link
Member

Choose a reason for hiding this comment

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

This is really lovely

Copy link
Member

@pauldambra pauldambra left a comment

Choose a reason for hiding this comment

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

Definitely worth a try. Or rather this should definitely work...

Ignoring heatmap this is the #1 support load from toolbar so 🏅 for you if this works

@benjackwhite benjackwhite added the bump patch Bump patch version when this PR gets merged label Oct 24, 2023
@benjackwhite benjackwhite marked this pull request as ready for review October 24, 2023 15:46
@benjackwhite benjackwhite merged commit 6dbe7e8 into master Oct 25, 2023
14 checks passed
@benjackwhite benjackwhite deleted the fix/toolbar-loading branch October 25, 2023 07:50
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.

2 participants