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

Add an option to add user identification in FlagsmithProvider #105

Closed
MarkLyck opened this issue Mar 22, 2022 · 4 comments
Closed

Add an option to add user identification in FlagsmithProvider #105

MarkLyck opened this issue Mar 22, 2022 · 4 comments
Assignees

Comments

@MarkLyck
Copy link

I tried migrating to the new flagsmith/react using hooks and the FlagSmithProvider.

However I am now running into frustrating race conditions on the identify...

Without the Provider/hooks I am able to identify my user as soon as they pass the Authentication step in my app (before React renders) and the first call to flagsmith ends up having the correct user identified flags

However with the new useFlagsmith hook that's suggested to identify a user with. This can only happen after React has rendered, and in a component at a lower level than the FlagsmithProvider.

That means now on the first render of my app and the first fetch of flagsmith flags, I get default values. Then when useEffect runs (which I had to put in a random nonsensical place due to the structure here) it fetches flags again with the right attributes.

This is a waste and frustrating to deal with.

Please let me identify the user via props in the FlagsmithProvider instead of the useFlagsmith hook. It will clean up my app and fix the race condition issue.

@kyle-ssg
Copy link
Member

kyle-ssg commented Mar 23, 2022

Hi, firstly I apologise if you found this frustrating to work with.

Without the Provider/hooks I am able to identify my user as soon as they pass the Authentication step in my app (before React renders) and the first call to flagsmith ends up having the correct user identified flags

It sounds like in this case, really the issue is more that you wish to initialise or identify separately to components / render lifecycle.

Considering what you've raised, I've done 2 things.

1: I've marked the Flagsmith provider options as optional, this means the flagsmith instance you pass into the provider can be initialised both before and after this is rendered. It sounds like this might be preferable for you if you were previously rendering the correct values on the initial render.

As a side bonus this also that means SSR applications will not need to pass options into the component, they basically have the same use-case as yourself where they fetch flags prior to renders.

flagsmith.init({
    environmentID: "QjgYur4LQTwe5HpvbvhpzK",
    cacheFlags:true
})

...
  <FlagsmithProvider flagsmith={flagsmith}>
    <App />
  </FlagsmithProvider>

2: I've added the ability to initialise with identity and also traits. This means if you know the identify beforehand you can pass it into init or as part of the Flagsmith provider options.


flagsmith.init({
    environmentID: "QjgYur4LQTwe5HpvbvhpzK",
    cacheFlags:true,
    identity: "my_user",
    traits: {{trait1:"value}}
})
...
  <FlagsmithProvider flagsmith={flagsmith}>
    <App />
  </FlagsmithProvider>


Or

  <FlagsmithProvider flagsmith={flagsmith}
    options={{
    environmentID: "QjgYur4LQTwe5HpvbvhpzK",
    cacheFlags:true,
    identity: "my_user",
    traits: {{trait1:"value}}
  }}>
    <App />
  </FlagsmithProvider>

You'll need to update to flagsmith or react-native-flagsmith ^2.0.5 to do this.

I'll shortly update the snippets in the flagsmith dashboard to inform users of this. Also, the docs for this will be published to docs.flagsmith.com later today.

If you have any questions regarding the above let me know / re-open the issue.

@MarkLyck
Copy link
Author

@kyle-ssg This is amazing! and thanks for the super fast turnaround! 😁 🎉

@kyle-ssg
Copy link
Member

No problem at all, I'm keen to use this on projects too so I want it to be the best! Thanks for taking the time to be active

@mrlubos
Copy link

mrlubos commented May 12, 2022

@kyle-ssg Please let me know if I should create a new issue, wanted to follow up on this conversation as I've just looked into upgrading to v2. I like the idea behind using a React native approach which makes Flagsmith similar to other React libraries.

I played with v2.0.9 for a bit and noticed a few things.

  1. This still makes a call to Flagsmith, which then unsurprisingly fails.
<FlagsmithProvider
  flagsmith={flagsmith}
  options={{
    environmentID: ''
  }}
>

Can you please make it so that we don't try to send any Flagsmith API requests when the ID is missing? There could be a console warning if you think it's valuable in case someone forgot to inject the correct variable. For my use case, it's not necessary, I just don't want to run Flagsmith in local development. Pre-v2, I would just not call flagsmith.init() to achieve this.

  1. Looking at the <FlagsmithProvider /> props, flagsmith={flagsmith} stands out. What is the purpose of importing it as import flagsmith from 'flagsmith' only to pass it to the provider? Can't you do that automatically for me? I assume this is intended for initialization outside components which may lead to faster first render, correct? For my use case, I'd prefer to pass all options to the provider, otherwise there's very little improvement to my existing implementation. I have my custom useFeatureFlag() hook which uses Flagsmith under the hood, so at the moment, v2 approach adds seemingly more complexity to implement at very small advantage (fewer re-renders)

Curious to hear your thoughts, thanks!

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

No branches or pull requests

3 participants