Add LD config option to specify list of relevant flags

[thomassvensen]

We have a problem with retiring LD flags. We have many short-lived flags, used to e.g. hide a feature under development. LD has a very nice functionality, showing when a flag was last retrieved by a client, which makes it feel much safer to delete/archive a flag. The problem is that the flopflip LD adapter uses allFlags when retrieving from LD. Therefore, flags always look "in use" from LD's perspective.

A nice solution would be if the LD configuration could take an additional, optional parameter, flags, which listed all the flags we want flopflip to retrieve and subscribe to. In our own code, we could then have a global list of all flags currently in use throughout our app. We would then refer to flag name definitions in this file whenever we requested a flag status from flopflip.

Looking at the code, it seems like a fairly small change inside getInitialFlags, using the ldclient.variation instead of ldclient.allFlags. Of course, it will need to receive a list of flag names to iterate over, and fall back to allFlags if the list is empty or null.

[tdeekens]

This sounds like a nice solution to ease flag maintenance, yes. It started to bother me too now that we have many flags. I think it should somehow be part of the adapterArgs as nor all support this feature and generally accepting and passing down a prop can lead to false expectations I suppose.

[thomassvensen]

As for #841 - would be awesome if you could look into an implementation for this, too 😉

[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

You would just pass flags to adapterArgs in the object form. So

const flags = {
  flagA: false,
  'flag-b': true
};

<ConfigureFlopFlip
  adapter={adapter}
  adapterArgs={{ clientSideId, user, flags }}
  render={() => <App />}
/>

Would invoke variant twice with flag-a and flag-b while defaulting them accordingly. More also on the respective PR.

[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

[thomassvensen]

Hi again, and thanks for providing this so quickly! But, there seems to be an issue when specifying the flags. Normally, Redux looks like this: But when specifying flags, we get this: And, not surprisingly, the flopflip selectors then do not return the expected flag status.

[tdeekens]

Sorry to hear. That‘s a bit odd. We are using it without Redux and it seemed to work fine so far. I will check again later this week. Do you have an idea what could be the case? What does your setup look like

[thomassvensen]

Hi again! I have tried with various versions, first passing flags as a list of string names

        adapterArgs={{
          clientSideId: config.launchDarklyClientId,
          flags: Object.values(flags),
        }}>

That is what I did in the picture above. Then I looked at your commit:

if (adapterState.client && flags) {
          flagsFromSdk = {};
          for (let [requestedFlagName, defaultFlagValue] of Object.entries(
            flags
          )) {
        ....

And after scratching my head a bit, I realized that I should pass an object looking {firstFlag: false, secondFlag: true, ...}. An example usage in the documentation would have been useful ;-). Not it works, but there is a problem with the subscription to changes. When turning a feature on/off, I have to reload the page for it to take effect. Are you not having that issue in your scenario?

[tdeekens]

Glad you got it to work. Yes, passing an object is what we need as variation on the LDClient receives a second argument being the default value. I didn‘t want to document it yet cause I wanted to test it in one of our apps first. Some adapter specific things just can not be tested here. I will get back to you.

[tdeekens]

I can confirm this is a bug. I think I know the cause. I knew I wanted to be careful with this automatic flag normalization :)

[tdeekens]

You can try out: @flopflip/launchdarkly-adapter@2.9.1

[thomassvensen]

Yes, now it works! Great work! Thank you!