PostHog / posthog-js-lite

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

Incorrect types on `personProperties` for `getAllFlags` #194

Open xmonkee opened 7 months ago

xmonkee commented 7 months ago

Bug description

personProperties is typed as Record<string, string> whereas it should be Record<string, string | number>

How to reproduce

Try to call getAllFlags with a numerical personProperty and you will get a type error

image

Related sub-libraries

Additional context

posthog-node version 3.6.3

marandaneto commented 7 months ago

The JS SDK is defined as:

export type Property = any
export type Properties = Record<string, Property>

@neilkakkar should it be similar to the JS SDK or was it intended to be <string, string>?

neilkakkar commented 7 months ago

oh I seem to have overlooked this. We can & should accept numbers too indeed 👍

marandaneto commented 7 months ago

oh I seem to have overlooked this. We can & should accept numbers too indeed 👍

@neilkakkar would you like/do you have time to take a stab at it before we release the next major? not sure if this is a breaking change, since Record<string, Property> will still be compatible with <string, string>.

neilkakkar commented 7 months ago

sigh want to but don't seem like I have the time with all the incident fixes :/

neilkakkar commented 7 months ago

Yeah wouldn't be breaking, will relax types in all functions