splitio / react-client

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

Issue with SplitFactoryProvider #186

Closed oferitz closed 9 months ago

oferitz commented 10 months ago

Upgrading to version 1.11.0 with SplitFactoryProvider causes the application to hang indefinitely. However, the same version (1.11.0) works with the deprecated SplitFactory. My custom provider code is as follows:

import * as React from 'react'
import { SplitFactoryProvider } from '@splitsoftware/splitio-react'
import { FullPageSpinner } from 'components/Loader'
import { useUser } from 'context/UserProvider'

const SplitProvider = ({ children }: React.PropsWithChildren) => {
  const user = useUser()
  return user.id ? (
    <SplitFactoryProvider config={{
    core: {
      authorizationKey: SPLIT_API_KEY,
      key: user.id
    },
    impressionListener: {
      logImpression(impressionData) {
        datadogRum.addFeatureFlagEvaluation(impressionData.impression.feature, impressionData.impression.treatment)
      }
    }
  }}>
     {children}
    </SplitFactoryProvider>
  ) : (
    <FullPageSpinner />
  )
}

export default SplitProvider

The child component listens to Split client events, and with the new provider, client.Event.SDK_READY is never called, nor is client.Event.SDK_READY_TIMED_OUT. Since these events are used to determine the loading state, the application hangs. The child component code is as follows:

import React from 'react'
import { useSplitClient } from '@splitsoftware/splitio-react'

const FeatureFlagsProvider = ({ children }: React.PropsWithChildren) => {
  const { client } = useSplitClient()
  const [splitReady, setSplitReady] = React.useState(false)

  React.useEffect(() => {

      client.once(client.Event.SDK_READY, () => {
        // doing some other stuff here....
        setSplitReady(true)
      })

      client.once(client.Event.SDK_READY_TIMED_OUT, () => {
         // doing some other stuff here....
        setSplitReady(true)
      })

      client.on(client.Event.SDK_UPDATE, someUpdateFunction)
    }
  }, [client])

  return splitReady ? <>{children}</> : <FullPageSpinner />
}

export { FeatureFlagsProvider }

Switching from SplitFactory to SplitFactoryProvider causes the application to hang indefinitely because splitReady is never set to true. No console errors or visible failures are apparent in the network.

EmilianoSanchez commented 9 months ago

Hi @oferitz

Thanks for sharing. I will first explain the reason for your issue, then make a refactor suggestion to fix it, and finally a few more code refactor suggestions.

There is a breaking change behaviour in SplitFactoryProvider with respect to SplitFactory that is causing your code to hang. I explained this change here, but we haven't included it in our docs. So I am sorry for that.

Basically, the SplitFactoryProvider creates the factory (and so, its client) in the "componentDidMount" method, while SplitFactory does it in the "constructor". This implies that in the 1st render of your components, the client and factory properties are null if using SplitFactoryProvider, while they are defined if using SplitFactory. In the second render, client property is defined but its SDK_READY event has already been emitted, and so your client.Event.SDK_READY callback is not invoked.

const FeatureFlagsProvider = ({ children }: React.PropsWithChildren) => {
  const { client, factory, isReady } = useSplitClient()

  // Inside a SplitFactoryProvider component, next line prints:
  // `null null false` in the 1st render
  // `[Object] [Object] true` in the 2nd render when SDK_READY event is emitted
  // Inside a SplitFactory component, next line prints:
  // `[Object] [Object] false` in the 1st render
  // `[Object] [Object] true` in the 2nd render when SDK_READY event is emitted
  console.log(client, factory, isReady);

  ...
}

You can refactor your second code snippet as follows: You don't need to listen for the client events, because the client readiness state is already accessible through the isReady and isReadyFromCache properties of the context:

import React from 'react'
import { useSplitClient } from '@splitsoftware/splitio-react'

const FeatureFlagsProvider = ({ children }: React.PropsWithChildren) => {
  const { client, isReady, isReadyFromCache } = useSplitClient();
  const splitReady = isReady || isReadyFromCache;

  return splitReady ? <>{children}</> : <FullPageSpinner />
}

export { FeatureFlagsProvider }

Some extra suggestions:

<SplitFactoryProvider config={myConfig} updateOnSdkUpdate={true} ><MyComponent /></SplitFactoryProvider>

const { client, ... } = useSplitClient({ updateOnSdkUpdate: true });
const { treatments, isReady, isReadyFromCache } = useSplitTreatments({ names: ['feature_1'], updateOnSdkUpdate: true /* false by default */ });
import * as React from 'react'
import { SplitFactoryProvider, SplitClient } from '@splitsoftware/splitio-react'
import { FullPageSpinner } from 'components/Loader'
import { useUser } from 'context/UserProvider'

const splitConfig = {
    core: {
      authorizationKey: SPLIT_API_KEY,
      key: 'anonymous' // to use when `user.id` is not defined
    },
    impressionListener: {
      logImpression(impressionData) {
        datadogRum.addFeatureFlagEvaluation(impressionData.impression.feature, impressionData.impression.treatment)
      }
    }
};

const SplitProvider = ({ children }: React.PropsWithChildren) => {
  const user = useUser()
  return (
    <SplitFactoryProvider config={splitConfig}>{
      user.id ?
        (<SplitClient splitKey={user.id} >{children}</SplitClient>) :
        (<FullPageSpinner />)
    }</SplitFactoryProvider>
  );
}

export default SplitProvider
oferitz commented 9 months ago

@EmilianoSanchez, thank you for the thorough explanation and additional suggestions! It really makes sense now. I will try to implement it according to your answer.

oferitz commented 9 months ago

Closing. The suggested solution is working.