[React] Disable suspense by default, add `useSuspenseFlag` hooks

[toddbaert]

Currently, the React SDK suspends by default. Suspense is a REALLY cool feature, and very useful for our library in particular. However, this means that users see errors if they don't set up a suspense boundary. I'm proposing we make 2 changes:

• disable suspense by default
• add dedicated "useSuspenseXxx" versions of all relevant hooks
  • the existing hooks could still be used with suspense, but you would have to pass { suspense: true} in the options
• mark all suspense features as experimental in jsdocs
  • from react 18 docs: Suspense-enabled data fetching without the use of an opinionated framework is not yet supported. The requirements for implementing a Suspense-enabled data source are unstable and undocumented. An official API for integrating data sources with Suspense will be released in a future version of React (https://react.dev/reference/react/Suspense#usage)

The above proposals are consistent with popular libraries that use suspense on data fetching:

TanStack:

• has useQuery and useSuspenseQuery; useSuspenseQuery is just useQuery-v4 with suspense enabled

Apollo:

• has useSuspenseQuery which is a "a Suspense-ready replacement for useQuery that lets you take advantage of React's Suspense features while fetching during render"

There is only a handful of libraries that actually support suspense on data fetching, since it's an experimental feature.

:warning: This would be a behaviorally breaking change (things would compile, but suspense options would need to be inverted to maintain the same behavior).

[toddbaert]

cc @beeme1mr @lukas-reining @tjosepo @thisthat @Kavindu-Dodan @luizgribeiro @thomaspoignant

[tjosepo]

I support the proposed changes.

As much as I like Suspense, I can't deny that it's a very new concept and most of the React community isn't fully accustomed to it yet. Requiring users of the library to use a Suspense boundary by default can be seen as a small frustration.

I like the way TanStack and Appollo separate their hooks in suspense and non-suspense version.

[moredip]

+1 on not doing it by default. I also agree on following the lead of TanStack (their approach) - they have a lot of experience when it comes to great developer ergonomics for React SDKs.

[beeme1mr]

+1 from me too. I ran into an issue when writing a React tutorial where the page hung because I hadn't registered a provider. Other SDKs simply no-op in that situation, but the no-op provider is never "ready" (by design), so the evaluation hook was forever in a suspended state.

I really like the Suspense feature but I think users should essentially opt into it when they're ready.

[toddbaert]

After partially implementing this, I'm questioning if the useSuspenseXxx hooks are worth adding. My concern is that adding these might add some confusion. As it stands, you can enable suspense at the context-provider: <OpenFeatureProvider suspend={...}> and override that per hook useFlag('my-flag', false, { suspend: ... });. These options overlap completely, so the override behavior is quite clear.

A new useSuspenseFlag() hook would only take a subset of these options: Omit<ReactFlagEvaluationOptions, 'suspend' | 'suspendUntilReady' | 'suspendWhileReconciling'>, and I wonder if the override behavior here is a bit less clear.

@moredip @beeme1mr @tjosepo am I being worried about nothing?

[moredip]

After partially implementing this, I'm questioning if the useSuspenseXxx hooks are worth adding. My concern is that adding these might add some confusion. As it stands, you can enable suspense at the context-provider: <OpenFeatureProvider suspend={...}> and override that per hook useFlag('my-flag', false, { suspend: ... });. These options overlap completely, so the override behavior is quite clear.

A new useSuspenseFlag() hook would only take a subset of these options: Omit<ReactFlagEvaluationOptions, 'suspend' | 'suspendUntilReady' | 'suspendWhileReconciling'>, and I wonder if the override behavior here is a bit less clear.

@moredip @beeme1mr @tjosepo am I being worried about nothing?

I think really the question is whether we want the person adding the useFlag/useSuspenseFlag hook to have to think about the fact that suspense is or is not in play. If we want them to think about that, then I think we should have the distinct hook there to remind them. If we want to make it feasible to drop suspense in at a later date without touching a bunch of code, then we should consider not having two different hooks

[tjosepo]

I don't find the useSuspenseXxx hooks to be confusing. It's a common practice in most libraries that support Suspense. Because the suspenseful version of hooks can return different values (ex. it doesn't make sense to return a isLoading/isPending boolean when using Suspense, because the value is sure to have been loaded once the hook returns) I think it makes sense to create a new function (even if we could use function overloading in TypeScript to achieve the same result, separate functions are a bit cleaner imo).

Because their names are longer than the non-suspense hooks, I expect people to not use them by default (because long name = scary looking and complex). The only ones who will use them are people who know about Suspense.

It's true that the hooks are redundant and can trivially be added in user-land. I wouldn't mind disabling suspense by default and opening another issue for adding the Suspenseful hooks if we want to discuss it further.

I think the only confusing part is that both suspenseful and non-suspenseful hooks return exactly the same value. I would expect the non-suspenseful hook to return a isLoading/isPending variable to help tell me if the feature flags are available or not.

[toddbaert]

I opened https://github.com/open-feature/js-sdk/pull/940 (@tjosepo I tried to add you as a reviewer but it seems I can't; did accept your org invite?)