Setup package for feature flags

[devongovett]

Problem: we need a way to develop new features for existing, non-pre release components like Table and Menu. We want to be able to release these new features for teams to test before they are stable, without breaking anything in existing apps.

Solution: feature flags! We'll keep them in the @react-stately/flags package as exported booleans. Any package that wants to put code behind these flags will import this package and check the flags in an if statement. This might mean enabling new things, or even swapping out the entire implementation of something. An app can also import this package, and call a function to to enable some features they want to try out.

This approach is the easiest because the flags can be accessed from anywhere. However, one downside is that flags are enabled for all instances of a component across the whole app, not for one specific table for example. We could do that with context, but then the flags would need to be wired to more places that aren't allowed to access classes (e.g. classes, non-hook functions, etc.). The flags also need to be set before anything is rendered in the app or things may break unexpectedly, so libraries like quarry should not be enabling flags, they should be enabled in apps. Like any other package, there must be only one copy installed or things will also not work.

It will be expected that we add and remove flags from this package over time. Once a feature is stable, the flag will be completely removed and all code will be non-conditional. Setting the value of that flag will do nothing. Thus, the type accepted by the setFlags function is intentionally vague (Record<string, boolean> rather than listing the names of the flags).

[rspbot]

Build successful! 🎉

• View the storybook
• View the storybook-17
• View the storybook-16
• View the documentation

[snowystinger]

I think flags should only be set once, especially since it's not state, so it likely won't cause the app to dump and reload.

So I think we should export a singleton instance, and the setter should only set once, and then freeze whatever object we store in the singleton, then we use a getter of some sort to check. This could help if any of our flags end up with dependencies on each other.

This will also prevent us from changing exports routinely

Do we want to allow more than booleans? I'm trying to think if any other data would be useful here. Fortunately this isn't a rollout mechanism, so we don't need to worry about dates/users/etc.

[devongovett]

let's start simple. we can always do more fancy things later but I don't think it's necessary to get too complex for now. I don't think modifying exports is necessarily bad. In fact ESM is specifically designed to allow this. It also enables flags for components that you don't use in your app to be completely tree shaken. Freezing will prevent flags from being set in different places within the app. For example, you might have one file that renders a Table and that one is what sets the flag for table, and another file which sets a flag for menu. I was about to update the API to allow that actually. I do think making it only allow enabling and not disabling makes sense though.

[snowystinger]

Sure, happy to start simple and nix most of those things. However, I think changing the flags, in any direction, after the very first step of loading your app, is a bad idea. For instance, if you have two tables, and you render the first one expecting no features, and then you render the second one and turn on the features. Now the first table may be in a bad state if say, the feature flag was choosing between two hooks or suddenly added a hook or different state objects. And to make it a little worse, you may not know that the first Table is in a bad state for a long time, maybe it won't manifest a problem until it re-renders, or maybe you'd need to click on a cell. It can be very difficult to debug.

[devongovett]

Yeah I know, that's why I wrote "The flags also need to be set before anything is rendered in the app or things may break unexpectedly, so libraries like quarry should not be enabling flags, they should be enabled in apps." in the description.

But freezing the object wouldn't prevent this. You could still set no flags before rendering, and only set them once afterward which would have the same problem. Anyway, what I have now is equivalent to this since you can now only set them from false to true and not the other direction.

[snowystinger]

We had two questions come in today alone where people did not read the documentation.

If they must all be set at the same time, I think that would increase the odds that people do it in the right place. It's not foolproof, for sure, but I think it could be helpful in deterring misuse.

I went looking to see if we could detect if the code was run during a react lifecycle or during render, but couldn't find anything for that. Also discarded doing it from our Provider because the aria/stately packages don't require our provider.

We can still improve on it, so I've approved. In the big picture, these are fairly small issues I have. I only worry about support, as always, 😢

[rspbot]

Build successful! 🎉

• View the storybook
• View the storybook-17
• View the storybook-16
• View the documentation

[rspbot]

## API Changes

unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }

@react-stately/flags

unavailableMenuItems

-
+enableUnavailableMenuItems {
+  returnVal: undefined
+}

tableNestedRows

-
+enableTableNestedRows {
+  returnVal: undefined
+}

---

[ktabors]

This is great! I'm wondering about how this would be used for something like unavailableMenuItems. Unavailable Menu Items requires focus changes, so are using this just where Menu implements Unavailable Menu Items or in the focus code as well? I'm trying to figure out if this is just for teams to turn on and off alpha/beta features or for us to say we're giving you this alpha feature which might cause problems with other features or components and you can turn off the change for stability in your application.

[devongovett]

Yeah it would be for all the code related to that feature, across all packages. Within reason I guess. Like if there are changes that are small and useful for other things, then maybe those wouldn't be.