TrueCar / react-launch-darkly

Simple component helpers to support LaunchDarkly in your React app.
MIT License
76 stars 20 forks source link

Support for hooks? #119

Open Brianzchen opened 4 years ago

Brianzchen commented 4 years ago

Any plans to support hooks so we can simplify our component tree or create easier composition of multiple feature flags? This feature has been around for more than a year, so it would be really great to support the modern standard.

Support would have to be bumped from React v16.3.0 to v16.8.0.

Suggested api

useFeatureFlag(flag: string): boolean | undefined
// undefined === 'loading'

Usage

import React from 'react';
import { useFeatureFlag } from 'react-launch-darkly';

const Comp = () => {
  const isAnniversaryCallTask = useFeatureFlag('anniversary-call-task');

  return (
    <div>
      {isAnniversaryCallTask ? 'Flag on' : 'Flag off'}
    </div>
  );
};
sethbattin commented 4 years ago

Hi @Brianzchen - certainly that would be a welcome addition! If you are willing to submit a patch, we could possibly roll it into the (growing) list of additions for the next major version number. We are using the recentmost incarnation of react context, so creating the hookish useContext form would be straightforward.

Just for note-taking purposes, here's the list of things we'd need in addition to the implementation:

  1. Tests covering the new code
  2. Documentation for the new API

A little more detail - for tests, keep in mind that the launch darkly API has a few lifecycle events, and we want to make sure the hook acts in a reasonable way during those. For documentation, the main readme is fine, though we might have to discuss wording a bit since we're introducing a fully alternate usage.


As a stretch goal, consider some refactoring along these lines in order to streamline the library - currently the main API uses class-based context. It might be simple to flip those components to also be a hook consumer. The only sticky part might be that the client initialization has to happen somewhere, and currently that occurs in FeatureFlagRenderer. Moving that up to the top-level context provider would require some delicate dancing with other hooks. For example, we probably couldn't useState directly, or we'd cause full rerenders every time a flag changed. We'd rather only the hook consumers rendered.

So that certainly seems like a lot of work, but it would probably be required in order to make a hook-only system functional regardless. So we might as well make the old API forward-compatible in the same motion. That would facilitate smooth upgrades in codebases where there's lots of existing usage.


tl;dr the idea is great, and it would be great to see a PR for it!