Type-safe feature flags

[glentakahashi]

Is your feature request related to a problem? Please describe.
We currently do this in our code:

launchdarkly-js-common-sdk.d.ts :

declare module "launchdarkly-js-sdk-common" {
  // eslint-disable-next-line @typescript-eslint/no-empty-interface
  export interface LDFlagSet {
    someFlag: boolean;
    anotherFlag: boolean;
  }
}

Which allows us to use interface merging in order to get autocompletion on feature flags.

However, since it only extends the existing interface ([key: string]: LDFlagValue), it's still just as easy to accidentally use a nonexistent flag in the code as any key returned is any. This means that when deleting a feature flag, its relatively easy to accidentally leave behind parts of code that still reference this key, as the typing still resolves to any.

Describe the solution you'd like
I would like there to be a convenient way to get type safety for feature flags by defining an override somewhere in the code. The best case scenario for this would be for LDFlagSet to be an empty interface by default, however that's obviously a huge breaking change. If that were the case though, it would make it so that we can strongly type our feature flags by doing the above declaration merging.

Describe alternatives you've considered
We've tried doing more complex typescript type resolution by excluding the builtin types and using our own, but I was never able to get it to work properly. There may be a way, but there a number of outstanding typescript issues here as well that don't give me hope e.g. microsoft/TypeScript#36146

[eli-darkly]

I can't imagine how this could possibly work. It might help if you could provide some kind of example of what you'd like your code to look like— I mean, not just how you would access the flag, but how you would "define an override somewhere in the code" to enforce this typing. I also can't tell if you're actually talking about typing as in the data type of the flag value (which can't really be enforced in code, since you can change the variation values to any type on the service side), or just the existence of a specific flag key (even though you can't prevent a flag from being deleted on the service side either).

[glentakahashi]

So the above works out of the box for us with typescript by defining the file above in the root project directory. An example in vscode:

You're right in that it doesn't fully protect, since the service and code can get out of sync, but it does greatly improve the out of the box experience. This keeps all of our feature flag definitions in a single file, so the manual effort to keep things in sync is greatly decreased.

This doesn't prevent the following though (fake flag is typed as any and doesn't create compiler errors):

As an alternative, what we ended up doing was this:

and then we enabled the eslint rules:

Which works decently well, but would still prefer a different method that happens within ts native instead of enforced by eslint rules

[eli-darkly]

I'm sorry, I'm having trouble following your example - it seems like the code sample you posted in the PR description doesn't match what you're saying in the most recent comment, so I'm not sure what you meant by "the above works out of the box for us with typescript by defining the file above". You've provided two different versions of launchdarkly-js-sdk-common.d.ts, and no example of what's in FeatureFlagSet.

[glentakahashi]

Oh sorry, i missed a file, FeatureFlagSet is just:

export interface FeatureFlagSet {
  "input-update-on-enter": boolean;
  "multiplayer-test-page": boolean;
  multiplayer: boolean;
  "data-connections": boolean;
  comments: boolean;
  "cache-default-app-state": boolean;
  "story-mode": boolean;
  notifications: boolean;
  runButtonInput: boolean;
  "single-monaco": boolean;
  "org-migration": boolean;
}

So restating my post in a more concise way:

We can get auto-completion for useFlags by adding an additional type definition for launchdarkly-js-sdk-common.d.ts in our src folder, and using declaration merging (https://www.typescriptlang.org/docs/handbook/declaration-merging.html#merging-interfaces) on the interface. However, declaration merging is purely additive, and still allows any other feature flag to be returned from useFlags with the type of any. It would be ideal if there were a way this could be even more typesafe, where referring to a missing feature flag in useFlags would return an undefined error instead.

As an alternative, we have just been using eslint rules to disallow using useFlags directly, and instead created our own useHexFlags that just casts useFlags using our typed feature flag files.

[codeBelt]

I would also like to useFlag to be typeable. You could do something like:

export function useFlags<F extends LDFlagSet>(): F {
  ...
}

This way you can do:

export interface MyFlags {
  addServicesRevenueDialog: boolean;
}

...

export const MyComponent: React.FC<IProps> = (props) => {
  const flags = useFlags<MyFlags>();

  // TypeScript knowns about "addServicesRevenueDialog"
  flags.addServicesRevenueDialog

  return ...
}

I would also suggest updating LDFlagValue to something like:

-export type LDFlagValue = any;

+export type LDFlagValue = boolean | number | string | undefined;

[eli-darkly]

Regarding the type definition of LDFlagValue, please see #16.

[mellis481]

+1 for the desire to make flags type-safe.

[odell-allstripes]

+1 on this issue. In particular the use of an index signature with a value of any is difficult to correct for. Since TS interfaces don't let you subtract an index signature, you can't supply a more specific type in your own declaration file.

Something like this should work but it doesn't, as all other string keys still have an any value on the interface:

declare module 'launchdarkly-js-sdk-common' {
  export interface LDFlagSet {
    testFlag: boolean,
  }
}

I agree re making useFlags hook generic. It's fine if the type defaults to the flexible LDFlagSet but that should not be hardcoded as the return value in LD module's useFlags.d.ts.

[TNonet]

Is there any update on this request?

[nieblara]

Also looking for an update on this request! Thanks!

[josephearl]

IMO a better solution here would be to add typesafe methods to LDClient like the Java client such as boolVariation, stringVariation etc that only accept default values of the correct type and return values of the correct type (throwing an error if the resolved value is not of the correct type)

[josephearl]

Raised a similar issue on the new repo launchdarkly/js-core#285

[josephearl]

This is available for testing launchdarkly/js-core#285 (comment)