react-launch-darkly icon indicating copy to clipboard operation
react-launch-darkly copied to clipboard

Expose a higher-order component version of FeatureFlag.

Open threehams opened this issue 8 years ago • 2 comments

The current API uses a component and callback which essentially forces the use of a class component. This is fine for smaller changes, but it doesn't compose well with other React HOC-based libraries.

An example with code splitting and conditional data fetching:

const CardGroup = compose(
  withFeatureFlag("useNewCard"),
  withRouter,
  fetchData("cards", props => ({ id: props.params.postId })),
  branch( // from recompose library
    props => props.featureFlags.useNewCard === "A",
    renderComponent(AsyncCardA), // code split
  ),
)(AsyncCardB); // code split

This can allow the A/B test to be completely transparent to the wrapped component, and it can be easily added or removed without adding custom wrapper components.

This should be possible to do while sharing code with the existing component, possibly as a simple wrapper.

threehams avatar Aug 09 '17 02:08 threehams

This is quite limited because it only allows for a single flag to be passed in, but it reuses the current FeatureFlag component to accomplish an HOC:

export default flagKey => WrappedComponent => {
  const _renderFeature = function(props, featureFlagValue = true) {
    const flags = {};
    flags[flagKey] = featureFlagValue;
    return <WrappedComponent {...props} featureFlags={flags} />;
  };

  return function(props) {
    return (
      <FeatureFlag
        flagKey={flagKey}
        // null flag value means LaunchDarkly is not initialized yet
        initialRenderCallback={_renderFeature.bind(this, props, null)}
        renderDefaultCallback={_renderFeature.bind(this, props, false)}
        renderFeatureCallback={_renderFeature.bind(this, props)}
      />
    );
  };
};

Thoughts?

peterbee avatar Apr 11 '18 18:04 peterbee

Thanks for resurrecting this issue @peterbee. I have one concern about your proposal:

return <WrappedComponent {...props} featureFlags={flags} />;

^- this tactic means that the wrapped component must comprehend a prop called featureFlags. That's undesirable because it means we can't take an existing feature, wrap it in the HOC (with no other changes), and see a different flagged outcome. Looking at @threehams's proposal via recompose, we do get that quality. The challenger component and the challengee(?) can be completely decoupled:

  1. start with existing feature
  2. invent challenger feature
  3. wrap existing feature usage with HOC that references challenger
  4. decide winner and delete the HOC

Although now that I'm looking twice at your code, it seems maybe that prop is superfluous anyway? We already know which callback was exercised by the featureFlagValue argument, so no need to pass the flag along to WrappedComponent. Right? We could still get an unmolested challengee in there.

Forgive me for making the same comment on both your posts, but please do submit a PR. It will make the code commenting a lot easier. :slightly_smiling_face:

sethbattin avatar Apr 12 '18 15:04 sethbattin