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

Identifier/trait confusion when using cacheOptions: { skipAPI: true } #227

Closed
rolodato opened this issue May 16, 2024 · 3 comments · Fixed by #229
Closed

Identifier/trait confusion when using cacheOptions: { skipAPI: true } #227

rolodato opened this issue May 16, 2024 · 3 comments · Fixed by #229
Assignees

Comments

@rolodato
Copy link

Using a slightly-modified version of the React example here, I can reproduce the issue below Flagsmith/flagsmith-js-examples@main...rolodato:flagsmith-js-examples-repro-17cf4148:main

I'm not sure what's going on here exactly, but you can see in some conditions we are using cached traits from a previous identity and inserting them into an unrelated identity. Identity B should never have a name: "A" trait set.

id-trait-confusion-react.mov
@rolodato rolodato assigned kyle-ssg and novakzaballa and unassigned kyle-ssg May 16, 2024
@kyle-ssg
Copy link
Member

If I understand this issue right, it has nothing to do with caching.

The problem is

  • Identifying with user a stores returned traits locally
  • calling identify again with a new identity combines previously set traits with the request for the new identity. It does this to allow for users to set traits prior to identifying, this is a common requirement.

The solution I think is that if we call identify and the sdk is already identified it should clear out the traits and just use whatever is provided at that time.

@kyle-ssg kyle-ssg linked a pull request May 17, 2024 that will close this issue
@kyle-ssg
Copy link
Member

@rolodato I believe this is resolved please start again with new identities without traits using flagsmith@4.0.2-beta-1

@matthewelwell
Copy link
Contributor

Yes, I've tested and the new version seems to have resolved the issue 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants