launchdarkly / react-client-sdk

LaunchDarkly Client-side SDK for React.js
Other
81 stars 67 forks source link

`asyncWithLDProvider` does not appear to support `deferInitialization` config option, despite accepting it #99

Closed BRMatt closed 2 years ago

BRMatt commented 2 years ago

We're trying to integrate the launchdarkly react SDK into our frontend react web app, but we're currently experiencing issues getting it to ignore anonymous users. We don't have, or need 'anonymous' users in our application - almost every page that launch darkly will be loaded on will be associated with a user key.

Our frontend app does not know the user key at the point where we initialize our react app, we have to call an API to fetch the key before we can pass it to the launchdarkly SDK.

While reading the docs we noticed the deferInitialization config option, which appears to do what we want:

This property allows SDK initialization to be deferred until the user property is defined.

By deferring SDK initialization, you defer all steps which take place as part of SDK initialization, including reading flag values from local storage and sending the SDK's ready event.

The one exception to this rule is that the SDK continues to load bootstrapped flag values as long as the bootstrapped values are provided as a map of flag keys and values. As mentioned above, if indicated that the SDK should bootstrap flags from local storage, this will not happen until the SDK initializes.

The part of our react app that integrates with launchdarkly is using asyncWithLDProvider() to create a provider component. It appears to take the same config options as withLDProvider(), but unlike withLDProvider(), it ignores the deferInitialization option.

It appears the option is only checked in provider.tsx:

https://github.com/launchdarkly/react-client-sdk/blob/5fe4ababa90a4815938f352a1071aa61671dc59a/src/provider.tsx#L71-L86

And asyncWithLDProvider does not appear to use this component:

https://github.com/launchdarkly/react-client-sdk/blob/5fe4ababa90a4815938f352a1071aa61671dc59a/src/asyncWithLDProvider.tsx#L27-L30

I found this a bit surprising as I couldn't see any documentation about it ignoring this option, and I found an example in a GitHub issue of using the config option with asyncWithLDProvider(). Is this a bug in the implementation, or a bug in the documentation/type definition for asyncWithLDProvider's config parameter?

Reading the code documentation for asyncWithLDProvider() I suppose I can see why it might not accept this option:

https://github.com/launchdarkly/react-client-sdk/blob/5fe4ababa90a4815938f352a1071aa61671dc59a/src/asyncWithLDProvider.tsx#L9-L18

But the fact that this behaviour/incompatibility isn't explicitly called out makes it seem like a bug.

Do you folks have any insight on whether this is a bug/expected behaviour?

yusinto commented 2 years ago

Hi @BRMatt, we are currently investigating this and will provide an updated response shortly.

yusinto commented 2 years ago

Hi @BRMatt, thank you for reporting this issue. It is by design that asyncWithLDProvider does not support deferInitialization because asyncWithLDProvider needs to be initialized at the entry point prior to render to ensure flags and the ldClient are ready at the beginning of the app. We are in process of updating our documentation to reflect this behavior.

bwoskow-ld commented 2 years ago

In addition to updating our documentation as @yusinto mentioned above, we have updated our types in version 2.23.3 to note that deferInitialization is deprecated and unsupported when used with asyncWithLDProvider.

BRMatt commented 2 years ago

thanks!