Add configuration option to auto-camelCase requested flag names

[thomassvensen]

We started using Launch Darkly before pulling in flopflip. In Launch Darkly, all our flags are defined as "kebab case", e.g. this-is-a-flag. Flopflip converts all of these to camelCase, e.g. thisIsAFlag, as you know. But we really want to keep flag names as kebab case in our code, as it much easier to mentally connect the code and LD when the casing is the same. It is also, for example, much easier to search for references inside the code when we can just copy/paste from LD, rather then have to convert casing.

Currently, we work around the above problem by wrapping every call to flipflop with a camelCase('this-is-a-flag').

A very nice solution would be if flopflip could do this for us. It could be a configuration parameter, or it could just be done for any received flag name. As flopflip always normalizes to camelCase after receiving from LD, it actually seems quite safe to just do the same when receiving requests from the client as well? But a (opt-in) config parameter is of course a safer route to avoid breaking any existing code.

[tdeekens]

Thanks for the issue. As flopflip supports multiple adapters the flag format can vary. As a consequence we decided to use camel casing initially. It felt more naturally to write e.g. when accessing flag values or keeping a flag name constant similar to its content. I agree thar in some caes, e.g. to lower the mental over head when looking at LD, kebab would be easier. An option would be to accept a casing argument for flopflip when configuring which continues to default to camel case to remain backwards compatible.

[jstclair]

I think the original point wasn't the internal storage, but rather when you request a flag:

<ToggleFeature flag={camelCase('our-flag-name')}>

or

injectFeatureToggles([
    camelCase('our-flag-a'),
    camelCase('our-flag-b'),
  ])

Just a quick look through the code definitions, it seems that perhaps adding a transform function to the AdapterArgs passed to createFlopFlipEnhancer (in this case, one could pass { transformer: camelCase } but the default could be identity ( transformer: input => input }. Naming is bad, since this would more accurately be getFlagNameTransformer

[tdeekens]

That sounds reasonable yes but I'd like to not have this burdan on the consumer of flopflip to repeat the "mapping into internal flag represenations".

I was wondering if something like:

<ConfigureFlopFlip
  adapter={adapter}
  adapterArgs={{ clientSideId, user, flagCasing: 'camel' | 'kebab' }}
  render={() => <App />}
/>

would do the trick. It would change the internal storage as a side effect but as a result the consumer would get flags exposed in the casing demanded.

[thomassvensen]

@jstclair was right about my intention, but I do like the solution you propose here @tdeekens

Is this something you would consider implementing, or would you prefer us to make a PR?

[tdeekens]

i can have a look over the weekend no worries.

[jstclair]

Is there a way to preserve kebab-casing on a JS object (other than converting it to a Set/Map)? Is the idea flags[‘key-name’] rather than flags.keyName?

On 16 Oct 2019, at 11:25, suǝʞǝǝpʇ notifications@github.com wrote:

 That sounds reasonable yes but I'd like to not have this burdan on the consumer of flopflip to repeat the "mapping into internal flag represenations".

I was wondering if something like:

<ConfigureFlopFlip adapter={adapter} adapterArgs={{ clientSideId, user, flagCasing: 'camel' | 'kebab' }} render={() => } /> would do the trick. It would change the internal storage as a side effect but as a result the consumer would get flags exposed in the casing demanded.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

[tdeekens]

I am not sure I follow. By eventually specifying casing: 'kebab' you would alter the API contract entirely. So you'd also do useFeatureToggle('my-toggle') over useFeatureToggle('myToggle') as the internals also change as to how flags are kept in memory in the adpater. As far as I understood that's what's intended.

Note also that there is a helper which I added to note if flag names passed are not camel cased: https://github.com/tdeekens/flopflip/blob/master/packages/react/modules/helpers/get-is-feature-enabled/get-is-feature-enabled.ts#L20-L20.

[tdeekens]

Ah, I think I finally get the point, sorry. You're suggesting to just "camel" case whatever is coming in. If it's already camel cased it won't change a thing. If it isn't it at least lowers the burdan on users not having to remember doing it themselves? I guess that also works. Shouldn't be a breaking change as far as I can tell?

[tdeekens]

Please test with the dist-tag published versions:

Successfully published:
 - @flopflip/launchdarkly-adapter@2.8.5-next.18+84c2cb9
 - @flopflip/localstorage-adapter@1.2.4-next.18+84c2cb9
 - @flopflip/memory-adapter@1.3.4-next.18+84c2cb9
 - @flopflip/react-broadcast@9.0.5-next.18+84c2cb9
 - @flopflip/react-redux@9.0.5-next.18+84c2cb9
 - @flopflip/react@8.0.5-next.18+84c2cb9
 - @flopflip/splitio-adapter@1.4.6-next.18+84c2cb9

[tdeekens]

Ok, integration tested and released. Closing. Please re-open and/or PR if issue are found:

Successfully published:
 - @flopflip/launchdarkly-adapter@2.9.0
 - @flopflip/localstorage-adapter@1.3.0
 - @flopflip/memory-adapter@1.4.0
 - @flopflip/react-broadcast@9.1.0
 - @flopflip/react-redux@9.1.0
 - @flopflip/react@8.1.0
 - @flopflip/splitio-adapter@1.5.0
 - @flopflip/types@2.2.0