feat: support type-safe variant values for InMemoryProvider

[MattIPv4]

This PR

Applies a few generics to the Flag and FlagConfiguration types used by InMemoryProvider to allow for type-safe variant values, ensuring the defaultVariant and any value returned by contextEvaluator are present as keys in variants for the given flag.

Related Issues

Resolves #967

Closes #1046

Notes

If folks are passing in an object reference to InMemoryProvider, like we are in the specs, rather than an object literal directly, this is technically a breaking change as they'll need to as const or pass it directly, to allow TypeScript to correctly infer the types. I'm personally of the opinion that this isn't a breaking change in the API of the package, and so this can ship as a minor change, but open to being told this should be considered breaking.

Also, the monorepo is using a very old version of TypeScript, 4.4, so I've had to include a util for NoInfer rather than using the built-in util that's available in TypeScript 5.4 and beyond.

Follow-up Tasks

N/A

How to test

See specs where this is already demonstrated to be working as I've had to apply @ts-expect-error when we're using invalid variant keys.

[gemini-code-assist]

Summary of Changes

Hello @MattIPv4, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the type safety of the InMemoryProvider by introducing generics to its core configuration types. The primary goal is to prevent runtime errors by ensuring that flag variant keys are consistently validated at compile-time, improving the reliability and developer experience when defining feature flags. This change makes the InMemoryProvider more robust and easier to use with strong typing.

Highlights

• Type-Safe Variant Values: Introduced generics to the Flag and FlagConfiguration types within InMemoryProvider to ensure type-safe variant values. This guarantees that defaultVariant and any value returned by contextEvaluator are valid keys within the variants object for a given flag.
• NoInfer Utility Type: Added a NoInfer utility type to maintain compatibility with older TypeScript versions (specifically 4.4, as used in the monorepo), as the built-in NoInfer is only available from TypeScript 5.4 onwards.
• Potential Breaking Change for Object Literals: Noted a potential breaking change for consumers passing existing objects to InMemoryProvider configurations. These objects may now require an as const assertion to allow TypeScript to correctly infer the types and leverage the new type safety.
• Enhanced Test Coverage: Updated existing tests to reflect the new type-safe behavior, including the strategic use of as const assertions and @ts-expect-error comments to validate both correct and intentionally incorrect type scenarios.

Changelog

• packages/web/src/provider/in-memory-provider/flag-configuration.ts
  • Introduced a NoInfer utility type for TypeScript version compatibility.
  • Defined a new FlagVariants type to represent type-safe variant records.
  • Updated the Flag type to be generic, accepting a type parameter T for variant keys, and utilized FlagVariants and NoInfer.
  • Modified the FlagConfiguration type to be generic, enforcing type safety across flag definitions.
• packages/web/src/provider/in-memory-provider/in-memory-provider.ts
  • Made the InMemoryProvider class generic to accept a type parameter T for its flag configuration.
  • Updated the constructor and putConfiguration method signatures to use the new generic FlagConfiguration type, propagating type safety throughout the provider.
• packages/web/test/in-memory-provider.spec.ts
  • Applied as const assertions to flag configuration objects in various test cases to enable stricter type inference.
  • Added @ts-expect-error comments to specific test lines where defaultVariant or contextEvaluator are intentionally set to invalid variant keys, verifying the new type-checking behavior.
  • Corrected casing for StringFlagSpec to stringFlagSpec in multiple test declarations for consistency.

Activity

• MattIPv4 created this pull request to add type-safe variant values for the InMemoryProvider.
• The changes address and resolve issue Type safety of Variant keys #967, which likely pertained to type safety concerns with flag variants.
• The author noted that existing InMemoryProvider consumers passing objects directly might need to add as const for proper type inference, but considers it a minor change.
• A custom NoInfer utility type was implemented due to the monorepo's use of an older TypeScript version (4.4), which lacks the built-in NoInfer.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces generics to the InMemoryProvider and its associated flag configuration types to enable type-safe variant values. This is a significant improvement, ensuring that defaultVariant and the return values from contextEvaluator are valid keys within the variants object. The implementation leverages advanced TypeScript features effectively, and the accompanying test updates are thorough, correctly using as const and @ts-expect-error to validate the new type safety. Overall, this is a well-executed feature enhancement.

[beeme1mr]

Hey @MattIPv4, thanks for the PR. It's nice a great improvement to the in memory provider. However, it appears to be a slight breaking change. Is this expected?

Type error

	const booleanFlagSpec = {
        'a-boolean-flag': {
          variants: {
            on: true,
            off: false,
          },
          disabled: false,
          defaultVariant: 'on',
        },
      }
      await provider.putConfiguration(booleanFlagSpec);
                                                               ^ Type 'string' is not assignable to type '"on" | "off"'.ts(2345)

No type error

Type error

	const booleanFlagSpec = {
        'a-boolean-flag': {
          variants: {
            on: true,
            off: false,
          },
          disabled: false,
          defaultVariant: 'on',
        },
      } as const;
      await provider.putConfiguration(booleanFlagSpec);

I understand why the const is important to the types but it would be nice if it fell back to the non-type safe version if the definition isn't a readonly type.

[MattIPv4]

Yeah, as I said in the notes, that is expected as a "breaking" change, though my view would be that it isn't actually a breaking change to the API that the SDK offers, it is simple enough for folks to add as const or pass a literal directly?

I'm not sure I'm keen on the idea of having it fall back to not being type safe if it cannot infer correctly, that feels like it'd end up being a DX trap for folks thinking it is acting in a type-safe way when actually it isn't because it silently failed to infer the types it needed?

[toddbaert]

Instead of modifying the existing putConfiguration method (which requires as const on all existing consumer code), what if we:

• Keep the current putConfiguration as-is, but mark it @deprecated.
• Add a new method (e.g. setConfiguration) that uses the generics from this PR to enforce variant key typing.
• In a future major version, remove the deprecated method.

This way existing consumers continue to work with zero changes on upgrade (no as const needed), new consumers and those who migrate get full type safety, and we avoid the "silent fallback" DX trap you rightly called out since the type-safe path is an explicit opt-in via a distinct method. The naming is flexible; the key idea is that we ship the type-safe method alongside the existing one rather than replacing it in-place.

I know this is a significant improvement, and if it would be great to just have it, but I can find any way to wiggle out of our breaking change contract on this one, since the memory provider is a part of the core SDK.

[MattIPv4]

🤔 That works for the putConfiguartion method, no concerns there, but it would still leave the constructor either with a breaking type change, or being unsafe with no clear way to solve that down the road outside a breaking change?

[toddbaert]

🤔 That works for the putConfiguartion method, no concerns there, but it would still leave the constructor either with a breaking type change, or being unsafe with no clear way to solve that down the road outside a breaking change?

hmmm we could use a factory function? I'm not in-love with that, but I don't know what other options we have. I don't think we can do this with a plain overload due to the way TS constructor overloads work.

I guess we could also make a subclass that adds all the type safety and deprecate the current one? WDYT about that?

[MattIPv4]

I guess we could also make a subclass that adds all the type safety and deprecate the current one? WDYT about that?

I think this is probably the cleanest solution to avoid a breaking change for now, users can opt in to the new class for type-safety, and when 2.0 happens, we can move the type-safety to the original class. Let me give it a shot 👍

[MattIPv4]

/gemini review

[gemini-code-assist]

Code Review

This pull request updates the InMemoryProvider to the new TypedInMemoryProvider across the Angular, Nest, React, Server, and Web SDKs to enhance type safety for flag configurations. This involved updating imports, adjusting constructor parameters, and adding as const assertions to flag configuration objects in various test files and examples. The review comments highlight two areas for improvement: the flagConfiguration parameter in the TestingProvider constructors for Angular and React SDKs should be given a default value to prevent type errors, and the README examples for the Server and Web SDKs incorrectly instantiate TypedInMemoryProvider without the new keyword.

[MattIPv4]

@toddbaert, how does this look now?

[toddbaert]

This looks good to me; keeping InMemoryProvider as-is (deprecated) and adding TypedInMemoryProvider alongside avoids the breaking change...

One thought on naming: once InMemoryProvider is eventually removed in a major version, TypedInMemoryProvider will sound a bit odd since there's no longer a non-typed alternative. Would something like InMemoryFlagProvider work instead? It can coexist but then stay on reasonably later.

Sorry for all the back and forth - I was going to just make this change myself, but I'd like your input. I'm going to approve either way. ❤️

[MattIPv4]

One thought on naming: once InMemoryProvider is eventually removed in a major version, TypedInMemoryProvider will sound a bit odd since there's no longer a non-typed alternative. Would something like InMemoryFlagProvider work instead? It can coexist but then stay on reasonably later.

My intention with the major would be to remove the old InMemoryProvider and switch TypedInMemoryProvider to being called InMemoryProvider?

[lukas-reining]

I think this is probably the cleanest solution to avoid a breaking change for now, users can opt in to the new class for type-safety, and when 2.0 happens, we can move the type-safety to the original class. Let me give it a shot 👍

I support this approach!

My intention with the major would be to remove the old InMemoryProvider and switch TypedInMemoryProvider to being called InMemoryProvider?

Maybe this is a good thing to put onto the 2.0 changes list.
To me this sounds like the way to go.

[lukas-reining]

This is a great addition!
Thank you @MattIPv4 and sorry for the delay!

I support switching the name after removing the untyped provider in v2.0.