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

fix: Handle uninitialised helpers better #767

Merged
merged 7 commits into from
Sep 7, 2023
Merged

Conversation

benjackwhite
Copy link
Collaborator

Changes

Currently if you call posthog methods before the init method is called, you get a lot of potential errors largely because we forcefully set the type to be defined.

A typical case for this is where you only intialise PostHog if in a production environment.

This makes the lazy options truly lazy, with some early exits and otherwise optional accessors to ensure we don't call things that would crash.

I think we need a much more concrete solution for this (maybe having a decorator type approach to make sure no method runs without the lazy loaded values existing (i.e. init being called)) but for now this should at least stop people's browsers from crashing.

Checklist

@benjackwhite benjackwhite added the bump minor Bump minor version when this PR gets merged label Aug 3, 2023
@benjackwhite benjackwhite marked this pull request as draft August 3, 2023 14:02
@github-actions
Copy link

github-actions bot commented Aug 3, 2023

Size Change: +6.38 kB (+1%)

Total Size: 684 kB

Filename Size Change
dist/array.full.js 177 kB +1.6 kB (+1%)
dist/array.js 119 kB +1.59 kB (+1%)
dist/es.js 118 kB +1.59 kB (+1%)
dist/module.js 119 kB +1.59 kB (+1%)
ℹ️ View Unchanged
Filename Size
dist/recorder-v2.js 93.6 kB
dist/recorder.js 58.3 kB

compressed-size-action

@benjackwhite benjackwhite marked this pull request as ready for review August 3, 2023 14:48
# Conflicts:
#	src/posthog-core.ts
src/posthog-core.ts Show resolved Hide resolved
src/posthog-featureflags.ts Outdated Show resolved Hide resolved
@posthog-bot
Copy link
Collaborator

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

@benjackwhite benjackwhite requested review from pauldambra and removed request for neilkakkar and mariusandra September 6, 2023 16:28
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.

clicked around locally and got events 👍

@pauldambra pauldambra merged commit 580a055 into master Sep 7, 2023
11 checks passed
@pauldambra pauldambra deleted the fix/optional-values branch September 7, 2023 10:15
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.

4 participants