splitio / react-client

React JS SDK client for Split Software
https://split.io
Other
27 stars 10 forks source link

[Feature Request] Support Suspense #192

Open tjosepo opened 5 months ago

tjosepo commented 5 months ago

Hi,

I hope you are having a great day where you are.

I've been trying the React SDK, and there's something I think SDK could really benefit from.

Problem

Currently, all the hooks of the client require the consumer to wait until the client is ready before trying to use their values.

This forces consumers of the library to code very defensively and add checks to isReady in every single component that uses Split.

Some use-cases, like setting a default value from a feature flag, become very difficult because of this.

function Component() {
  const { treatments, isReady } = useSplitTreatments({ names: ["isDefaultEnabled"] });

  // 💫 Oops! We forgot to check `isReady`! This inital value might not be what we expect!
  const [enabled, setEnabled] = useState(treatments["isDefaultEnabled"] === "on");

  // ...
}

Proposal

Suspsense data-fetching could solves this problem by making the component suspend until the client is ready. This way, the user never has to check if the client is ready -- if the component successfully renders, the client is guaranteed to be ready.

This could be implemented in a new hooks, called useSplitTreatmentsSuspense.

function Component() {
  const { treatments } = useSplitTreatmentsSuspense({ names: ["isDefaultEnabled"] });

  // If the component didn't suspend here, it means the client is ready, and we can use treatments directly.

  // ✅ This becomes valid!
  const [enabled, setEnabled] = useState(treatments["isDefaultEnabled"] === "on");

  // ...
}

Under the hood, the useSplitTreatmentsSuspense would throw a promise and suspend when the client is not ready. This promise would resolve once the client becomes ready, resuming the rendering of the component.

Unfortunately, there is currently no way to implement Suspense data-fetching in user-land when using the React SDK. This is because the Split client is completely hidden from the consumers of the library until the client is ready, preventing the user from listening to the activation event of the client and implementing useSplitTreatmentsSuspense themselves. This forces users who want to use Suspense to fallback to the JavaScript SDK.

Let me know what you think about this proposal, and let me know if I can help with anything.

agustinona commented 5 months ago

Thanks @tjosepo for the suggestion. Our team will review this.

This is because the Split client is completely hidden from the consumers of the library until the client is ready

Could you clarify what you mean here? You can use the useSplitClient hook to get the underlying JS SDK client and make use of its events. You may also create a JS SDK factory instance using the SplitSdk function (as described in the For more flexibility section here) to leverage the the JS SDK factory while still using the React package.

Do neither of these approaches allow you to implement your suspense hook?

tjosepo commented 5 months ago

You can use the useSplitClient hook to get the underlying JS SDK client and make use of its events.

The useSplitClient() hook returns { client: null } until the client is ready. This prevents listening to the SDK_READY event, since the client is only accessibly once it's ready. Without being able to listen to the SDK_READY event, we can't use Suspense, because we have no way to know when to resume rendering.

I don't know if this is by design or if this is a bug in the library. If useSplitClient() returned the client during initialization, it would actually let users implement their own Suspense hook, but because it doesn't, users can't.

That being said, I also think there is value in officially supporting Suspense in the SDK, rather than require users to create their own custom hooks given that Suspense is a first-class feature in React.

EmilianoSanchez commented 5 months ago

Hi @tjosepo ,

You are right. When SplitFactoryProvider is called with the config prop, rather than the factory prop, the client is null on the first render. This was done this way by design because we have to avoid side-effects in the render phase, as React component rules state. So the Split factory is created when the component did mount, and therefore the client is not available during initialization (1st render).

This is something that we will review for next breaking change release for sure, to make the client available on initialization and support Suspense. But we need to discuss it with the team, for instance, to decide if we should provide a new hook as your proposal, or a new suspense: true option for useSplitTreatments, etc. So, we don't have any ETA yet.

In the meantime, you can consider using the factory prop with the SplitSdk function mentioned by @agustinona, described in the For more flexibility section here.

Using the factory prop, client is always available. So the useSplitTreatmentsSuspense hook could be implemented as follows:

import { SplitFactoryProvider, SplitSdk, useSplitTreatments, IUseSplitTreatmentsOptions } from '@splitsoftware/splitio-react';

function shouldSuspend(client: any) {
  const status = client.__getStatus();
  return !status.isReady && !status.isReadyFromCache && !status.hasTimedout;
}

function useSplitTreatmentsSuspense(options: IUseSplitTreatmentsOptions) {
  const result = useSplitTreatments(options);

  if(shouldSuspend(result.client)) {
    throw new Promise<void>((resolve) => {
      result.client!.once(result.client!.Event.SDK_READY_TIMED_OUT, resolve);
      result.client!.once(result.client!.Event.SDK_READY_FROM_CACHE, resolve);
      result.client!.once(result.client!.Event.SDK_READY, resolve);
    });
  }

  return result;
}

function MyComponent() {
  const { treatments } = useSplitTreatmentsSuspense({ names: [MY_FEATURE_FLAG] });

  return treatments[MY_FEATURE_FLAG].treatment === 'on' ? 
    <MyComponentOn /> : 
    <MyComponentOff />;
}

const mySplitFactory = SplitSdk(MY_SPLIT_CONFIG);

export const App = () => {
  return (
    <SplitFactoryProvider factory={mySplitFactory} >
      <Suspense fallback={<Loading />}>
        <MyComponent />
      </Suspense>
    </SplitFactoryProvider>
  );
}

And thanks for highlighting the importance of supporting Suspense :)