launchdarkly / react-client-sdk

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

`ProviderConfig.flags` keys should use the same casing as output flags #134

Closed hon2a closed 1 year ago

hon2a commented 2 years ago

When initializing LaunchDarkly with asyncWithLDProvider it's possible to add flags as part of the config. The result is twofold:

However, it turns out that while the React SDK transforms the flags to kebab-case by default (e.g. output from useFlags), it still expects the flags in initialization config to be snake-cased. This was unexpected for me and took me quite a while to figure out, so I'd suggest that one of the following might help future users. Either

  1. expect flags in config to be camel-cased while useCamelCaseFlagKeys is true (default), or
  2. improve documentation to specifically clarify this point.

Thanks for your time.

eli-darkly commented 2 years ago

I think changing the documentation would be the right move, at least in the current major version. Even though the current behavior is confusing, changing it now would be a breaking change for anyone who was relying on it (and I don't think we could just call it a bug to be fixed in a patch version, because the current behavior isn't an arbitrary mistake in an implementation detail, it is logical in terms of those being the actual flag keys that exist by LD; it's just not documented correctly).

XieX commented 1 year ago

This has been documented here: https://docs.launchdarkly.com/sdk/client-side/react/react-web#configuring-the-react-sdk

The flag keys must be in their original form as known to LaunchDarkly rather than in their camel-cased form.

This is indeed working as designed, they are supposed to be the flag keys as created in LD. They can be any casing FWIW, they don't have to be kebab case.

I see it's not explained in the SDK's API docs though, I'll see what I can do about that 👍

XieX commented 1 year ago

Those docs were updated, hopefully it's a bit more clear now.

hon2a commented 1 year ago

Thanks, looks quite clear to me.