Ergonomic changes? Or maybe I'm missing something

[from-nibly]

Really excited about flagsmith. Just getting started with react.

I'm glad there are hooks available but right now I'm finding myself writing a wrapper hook to make it work better in my project. Am I ttly doing this wrong?

As far as I understand I only want to identify the user once, start listening, and subscribe once. Then throughout the app I might grab the value of feature flags.

But right now I've written a hook to ensure things are initialized, and is listening, and then another hook that uses that to just grab a value. Am I missing something?

I would like to just be able to do something like

const myFeature = () => {
  const {value, error} = useFlag('my_flag_name');
  
  if (error) {
    return <>ERROR: {error.toString()}</>
  }
  if (value == undefined) {
     return <>Loading</>
  }
  
  // use the value in some part of the component here.
};

Is there a reason that the initialization, subscribing, and identification, and getValue is all together in the same hook?

[TheMagoo73]

Hi, thanks for getting in touch, and using the project!

The logic was placed together as it really all serves a single cohesive purpose, so I wanted it modeled as single entity; it's a pattern used in a number of similar hooks (notably the Auth0 React library). Mostly it removes the need for potentially complex interactions between one hook holding the connection and state and a separate one for holding the actual flags - I think that could lead down a path to lower cohesion and higher coupling which is generally worth avoiding.

If you're able to share your code I'd love to take a look.

JC

[from-nibly]

This is what I have so far.

It's pretty rough. but this is the basic idea. I might switch out the null values for an isReady flag? or something IDK.

import { createState, useState } from '@hookstate/core';
import { Flagsmith, useFlagsmith } from 'flagsmith-react';
import { useEffect } from 'react';
import { useCurrentOrg } from '../api/orgs';
import { useOwnUser } from '../api/users';
import { topLevelError } from '../errors';

interface FlagResult {
  value: null | string | number | boolean;
  enabled: null | boolean;
}

type FlagsmithIdentified = Omit<
  Flagsmith,
  | 'isIdentified'
  | 'isLoading'
  | 'identify'
  | 'isListening'
  | 'startListening'
  | 'isError'
> & { isReady: boolean };

const identityLock = createState<'waiting' | 'identifying'>('waiting');

export const useFlagsmithIdentified = (): FlagsmithIdentified => {
  const {
    isLoading,
    isError,
    isIdentified,
    identify,
    startListening,
    isListening,
    ...rest
  } = useFlagsmith();

  const { data: user, error: userError } = useOwnUser();
  const { data: org, error: orgError } = useCurrentOrg();
  const identityState = useState(identityLock);

  useEffect(() => {
    if (
      !isLoading &&
      !isIdentified &&
      identityState.value === 'waiting' &&
      user &&
      org
    ) {
      identityState.set('identifying');
      console.log('identifying');
      identify(user.id.value, {
        org: org.name.value,
      }).then(() => {});
    }
  }, [identityState, isIdentified, user, org, isLoading]);

  useEffect(() => {
    if (!isListening) {
      startListening(10_000);
    }
  }, [isIdentified, isListening]);

  if (isError || userError || orgError) {
    console.log('Flagsmith error', isError || userError || orgError);
    topLevelError.set(
      (
        (isError && 'Flagsmith failed to load') ||
        userError ||
        orgError
      ).toString()
    );
    return {
      isReady: false,
      ...rest,
    };
  }

  if (isIdentified && isListening) {
    return {
      isReady: true,
      ...rest,
    };
  }

  return {
    isReady: false,
    ...rest,
  };
};

export const useFlag = (flagName: string): FlagResult => {
  const { subscribe, getValue, hasFeature, isReady } = useFlagsmithIdentified();
  const value = useState<null | string | number | boolean>(null);
  const enabled = useState<null | boolean>(null);

  useEffect(() => {
    if (value.value === null && isReady) {
      value.set(getValue(flagName));
    }

    if (enabled.value === null && isReady) {
      enabled.set(hasFeature(flagName));
    }
  }, [isReady]);

  subscribe(() => {
    value.set(getValue(flagName));
    enabled.set(hasFeature(flagName));
  });

  return { value: value.value, enabled: enabled.value };
};

I can then, in my codebase, simply run

const {enabled: appsEnabled} = useFlag('apps_enabled');

if (appsEnabled === true) {

};

Note: I'm currently refactoring this because it seems to be creating way more than one identification call.

But I see 2 different pieces of the flagsmith library.

1. Loading, identifying, resolving errors, creating a subscription to changes, removing subscriptions, removing identity, etc.
2. Getting the current value / enabled state of flags.

If I were to use the useFlagsmith() hook all over my codebase I would have to check if it was loaded, had some error, and been identified elsewhere every time I wanted the value of a flag.

So I think the result is that pretty much every user of the library would want to wrap this hook somehow in one place in their codebase to handle the initialization, and then somehow expose the values that come out of it.

Now I agree that using useFlagsmithFlag('my_flag_name_here') would introduce coupling between an app and flagsmith itself. However, that would easily be remediated with a file containing a list of hooks, one for each feature. which would then become useMyFlagNameHereFeature(). Now you aren't coupling, and you are also removing duplication of strings throughout the app.

I guess one argument could be "just use some sort of state management to pass around feature values". At that point however, I'm not sure what this library is providing. Especially when considering that the point of using a ContextProvider is to allow for usage of a context in many places. So if you use a state management library to store the flag data and just use the hook in one place you don't need a context, at which point you might as well just use the base SDK.

Ultimately while I'm coding in my app I want to be able to subscribe to the value / enabled state of a flagsmith feature in a component without having to worry about whether or not the flagsmith libraries complex initialization process is complete.

I would think that either the context provider itself (or my own piece of the code) would be responsible for initializing the flagsmith instance and then provide that as context to the rest of the app. at which point the only thing a hook subscribing to that context would provide is feature flag specific APIs.

One option would be to expose functions / properties on the context provider to allow for initialization and by configuration either

1. Prevent loading the app until either a cached version of flags is present or the initial server response is received.
2. Just return null/undefined for all flag values asked for until the initial server response is received.

This could be conceived as something such as

<FlagsmithProvider
  environmentId={FLAGSMITH_ID}
  cacheFlags={true}
  defaultFlags={{
    apps_enabled: { enabled: false },
  }}
  identify={async () => {
    const user = await getUser(); 
    return identify(user.id, { org: user.org.name });
  }}
  listen={10_000}
  onError={(err) => alert(err.toString())}
>
  {children}
</FlagsmithProvider>

Anyway that's quite a lot. Would love to know your thoughts so far.

[tm1000]

@from-nibly I'm running into the same issue as you and what you've created is almost exactly what I need

Additionally having defaultFlags can be used in SSR modes as well

[from-nibly]

Feel free to yoink the code. Unfortunately I probably wont have enough time to turn it into a library or anything like that.

[tm1000]

@from-nibly It seems sort of redundant to make it into a library (that extends this library only to extend the base flagsmith library)

I'd be curious if @TheMagoo73 has more thoughts as to your ideas because I think that's how this library should work

[TheMagoo73]

Hi all, sorry for the lack of input - the 'real world' has been a bit crazy of late!

I'll be honest and say that I created this in a bit of a rush to solve a very specific need on a project I was working on. It was something of a learning exercise (I'm new to React), and was needed yesterday... like so much in life lol.

Right... poor excuses out of the way, let's fix stuff! 😃

I've been thinking about a v2 for a while; the initialisation/identification/subscription process works in our initial use case. We can handle it in the top level component, so children know it's all good to go when they render. However, I've just started a NextJS side project, and I'm running into the same issues you're seeing. Far too much management of what should be going on 'behind the curtain'.

@from-nibly I very much like your idea of having a useFlag() hook, I think it would make the code more usable. And default flags are a must.

I should get some down time this weekend, I'll start bouncing some ideas around... Watch for a V2 branch...

[tm1000]

However, I've just started a NextJS side project

👋 I've been working in NextJS for 2.5 years now. I just started writing Flagsmith/flagsmith-php-client#5 and additionally https://github.com/clearlyip/laravel-flagsmith and have run into similar issues on the Frontend as mentioned here.