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

Incorrect types on personProperties for getAllFlags #194

Open
1 of 4 tasks
xmonkee opened this issue Feb 27, 2024 · 5 comments
Open
1 of 4 tasks

Incorrect types on personProperties for getAllFlags #194

xmonkee opened this issue Feb 27, 2024 · 5 comments
Labels
bug Something isn't working nodejs

Comments

@xmonkee
Copy link

xmonkee commented Feb 27, 2024

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

  • All of them
  • posthog-web
  • posthog-node
  • posthog-react-native

Additional context

posthog-node version 3.6.3

@xmonkee xmonkee added the bug Something isn't working label Feb 27, 2024
@marandaneto
Copy link
Member

marandaneto commented Feb 28, 2024

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
Copy link
Contributor

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

@marandaneto
Copy link
Member

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
Copy link
Contributor

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

@neilkakkar
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working nodejs
Projects
None yet
Development

No branches or pull requests

3 participants