PostHog / posthog-js-lite

Reimplementation of posthog-js to be as light and modular as possible.
https://posthog.com/docs/libraries
MIT License
69 stars 36 forks source link

fix: PostHogProvider initialization logic #206

Closed AdamDorwart closed 8 months ago

AdamDorwart commented 8 months ago

Problem

A regression introduced by #189 requires a client and apiKey to initialize a PostHogProvider even though the error message says only one is required.

Changes

The error condition logic is fixed and a non null assertion is made.

Release info Sub-libraries affected

Bump level

Libraries affected

Changelog notes

AdamDorwart commented 8 months ago

nbd, it happens 😄

I'm pretty fresh to Typescript. How would you prefer I resolve:

Forbidden non-null assertion  @typescript-eslint/no-non-null-assertion
     return client ?? new PostHog(apiKey!, options)

I could just

    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion

This also works but I'm not sure if there's a more idiomatic way to go about it without the throw at the end.

    if (client) {
      return client
    } else if (apiKey) {
      return new PostHog(apiKey, options) 
    }

    throw new Error('Type logic error: This should never happen')
marandaneto commented 8 months ago

@AdamDorwart I fixed the linting issue by just adding apiKey ?? '' since this code path won't be triggered anyway, less code and easier to read, also added the changelog. Thanks!