statsig-io / react-sdk

An SDK for using Statsig Feature Management and Experimentation platform in React js clients
ISC License
6 stars 6 forks source link

isLoading starts in false, flips to true, then flips back to false #17

Open dbalatero opened 9 months ago

dbalatero commented 9 months ago

I'm seeing an issue with the useGate hook where isLoading starts in the false state.

To reproduce, add this to a React component:

const myGate = useGate('some_gate_you_have');
console.log(myGate):

I see the following print statements:

{isLoading: false, value: false}
{isLoading: true, value: true}
{isLoading: false, value: true}

Problems

  1. Since isLoading starts as false, there's no way to block a render until a gate value comes back for real.
  2. possibly non-issue value flips to true while isLoading is also true. This is a little odd to see the value change, but it might be because you're doing 2 setState calls in a row.
daniel-statsig commented 9 months ago

Thanks for reporting @dbalatero. Are you able to share the full snippet of how to reproduce the issue as well as the version you are seeing this on?

I attempted to repro using v1.34.0 but am seeing it default to isLoading:true.

Here is my snippet:

import { StatsigProvider, useGate } from "statsig-react";
import "./App.css";

var states: unknown[] = [];

function GateOne() {
  const gate = useGate("a_gate");
  states.push(gate.isLoading);
  return (
    <div>
      a_gate: {gate.value ? "Pass" : "Fail"} {JSON.stringify(states)}
    </div>
  );
}

function App() {
  states.push("Start");

  return (
    <StatsigProvider
      sdkKey="client-111111"
      user={{ userID: "a-user" }}
    >
      <GateOne />
    </StatsigProvider>
  );
}

export default App;

This results in a page with:

a_gate: Pass ["Start",true,true,false]
dbalatero commented 9 months ago

I can try to repro it again and figure out how.

Before I do that, one existential question I have pertains to the TypeScript shape I found for useGate's return type:

https://github.com/statsig-io/react-sdk/blob/2fb511f8cb7325f49c3f6cbcd45ad6216947a88a/src/StatsigHooks.ts#L11-L17

The comment says Returns the initialization state of the SDK and a gate value – does isLoading mean:

  1. We're waiting on the entire SDK to load & initialize once, and once it's loaded it will always be false? OR
  2. We're waiting on this particular gate to resolve to a value, and once we get a response it will flip from true -> false?

I initially interpreted isLoading as option 2, but I think it might be option 1?

dbalatero commented 9 months ago

Oh and to answer your version question, I'm on 1.27.3.

daniel-statsig commented 9 months ago

We make a single network request to fetch all the gate/experiment values for the given user. isLoading is representing that network request, and will resolve to false once the network response is processed.

It is set here: https://github.com/statsig-io/react-sdk/blob/main/src/StatsigHooks.ts#L19-L45

Which comes from the StatsigContext: https://github.com/statsig-io/react-sdk/blob/main/src/StatsigProvider.tsx#L185 https://github.com/statsig-io/react-sdk/blob/main/src/StatsigProvider.tsx#L221

marceloclp commented 6 months ago

I am guessing anyone using StatsigSynchronousProvider in React Strict mode will run into the same issue.

This happens because of this useEffect here:

useEffect(() => {
    if (firstUpdate.current) {
      // this is the first time the effect ran
      // we dont want to modify state and trigger a rerender
      // and the SDK is already initialized/usable
      firstUpdate.current = false;

      if (typeof window !== 'undefined') {
        window.__STATSIG_SDK__ = Statsig;
        window.__STATSIG_JS_SDK__ = StatsigJS;
        window.__STATSIG_RERENDER_OVERRIDE__ = () => {
          setUserVersion(userVersion + 1);
        };
      }
      return;
    }
    // subsequent runs should update the user
    setInitialized(false);
    Statsig.updateUser(user).then(() => {
      setUserVersion(userVersion + 1);
      setInitialized(true);
    });
  }, [userMemo]);

Because of Strict mode, the component is rendered twice, and the effect will run on mount, setting initialized to false and calling updateUser - which I am guessing is what is hitting /initialize.

shutdownOnUnmount is also broken on Strict mode.

For now, the best option I've found is setting localMode to true.