Components not always re-rendering when using useGlobal() without an argument.

[m4ttheweric]

I am encountering an issue where a component is not always re-rendering correctly depending on how I access a state property. For example, if I have a property foo on my global state or a provider, there are two ways to access it:

"direct" way:

const [foo, setFoo] = useGlobal('foo');
//then use "foo"

"indirect" way:

const [state, setState] = useGlobal();
//then use "state.foo"

Under some circumstances, the "indirect" way won't cause the component to re-render. And oddly, just by merely adding a declaration of the state property the "direct" way, all of the issues go away, without changing any of the references.

Also, it seems there are some differences in outcome based on whether we are using a dispatcher vs. manually updating the state. But not always... (I know).

Also, it seems that simply updating any regular React vanilla useState value in the component will cause a re-render, and all of the state updates we were not seeing magically appear.

So, if you're a bit confused, don't worry, I was too. I have spent hours narrowing this down, and thankfully I was finally able to repro it very simply in a code sandbox:

https://codesandbox.io/s/reactn-bug-z7ebn

Let me know if you have any questions, but the sandbox lays out all the details.

Matt

P.S. I reported this several months ago, but didn't have time to isolate it. So, apologies for the extra ticket.

[quisido]

Is your issue addressed by the last comment in #137 ?

[m4ttheweric]

@CharlesStover No, unfortunately not. I am not using any useEffect calls in my example. So, I don't think that's what's going on. Unless I am missing something. If you take a look at my codesandbox and see that I am mistaken, let me know!

[m4ttheweric]

@CharlesStover Any update on this?

[quisido]

I can verify that it is occurring in your Code Sandbox example (thank you very much for providing!), and I am simply trying to find time to prioritize the fix. In the meantime, I would advise against using useGlobal() without a parameter if you are experiencing this issue.

[m4ttheweric]

Alright, thanks! I was going to see if I could figure out the issue. If I get anywhere, I'll make a pull request.

For now, the issue can be worked around by importing all the individual state members that you use from the entire state. Just having them in the component makes that "indirect" global state work fine like so:

//reference this in my component for everything...
const [state] = ChatState.useGlobal();

//just importing the individual members, makes the "state" work perfectly:
const [messages] = ChatState.useGlobal('messages');
const [isSendingMessage] = ChatState.useGlobal('isSendingMessage');
const [seenStatusMessage] = ChatState.useGlobal('seenStatusMessage');
const [showDetail] = ChatState.useGlobal('showSeenStatus');

[m4ttheweric]

So, I took a little time to investigate. I ended up in ./src/use-global.ts looking at lines 57-78. There's a comment that caught my eye over what appears to be an effect that cleans up a listener involved with "forcing" re-renders on state changes. The comment and code is:

// If this component ever updates or unmounts, remove the force update
//   listener.
useEffect((): VoidFunction => removeForceUpdateListener);

If am an understanding, the above effect is going to run the cleanup after each update and on unmount. And my hypothesis here is that this is wrong, since we want the listener to only get removed when the component unmounts so that subsequent state changes will execute the forceUpdate method and trigger the component to re-render. And maybe if this might explain why in my sandbox, the first time you do something it works, but then fails on subsequent attempts.

So, I added an empty dependency array to the useEffect call on the same line (line 63 is the change):

  // Return the entire global state.
  if (typeof property === 'undefined') {

    
    // If this component ever updates or unmounts, remove the force update
    //   listener.
    useEffect((): VoidFunction => removeForceUpdateListener, []);
    //I added an empty dependency array so the listener 
    //will only remove when the component unmounts

    const globalStateSetter = useCallback(
      (
        newGlobalState: NewGlobalState<G>,
        callback: Callback<G> | null = null,
      ): Promise<G> =>
        setGlobal(globalStateManager, newGlobalState, callback),
      [],
    );

    return [
      globalStateManager.spyState(forceUpdate),
      globalStateSetter,
    ];
  }

Now, I made the same change in the build I am using in my project and it seems to have resolved everything. But I am really not sure of unintended consequences as I don't fully comprehend if there's some reason why it should be removed on update and not just on unmount.

What I can say is that it seems to be related to the forceUpdate method not running after state changes. When I manually import and run useForceUpdate in my component after state changes, everything is happy.

My concern is that perhaps there's a good reason to remove the listener on updates and this is creating some kind of memory leak that adds a bunch of listeners without the correct cleanup.

I am curious what you think and if this is a fix or perhaps not quite.

[quisido]

First, thank you so much for taking the time to investigate this. You have saved me a lot of effort in my otherwise unfortunately busy schedule. I appreciate the deep dive, and I find it to be a remarkable quality of a developer to do such a task for an open source project one didn't create themselves.

And my hypothesis here is that this is wrong, since we want the listener to only get removed when the component unmounts so that subsequent state changes will execute the forceUpdate method and trigger the component to re-render.

We want to remove update listeners every re-render as well. Here's why:

function MyComponent() {
  const [global] = useGlobal();
  if (global.isBlue) {
    return <div>{global.numberOfBlueFish}</div>;
  }
  return <div>{global.numberOfRedFish}</div>;
}

In the above example, the component displays either the number of red fish or blue fish, and which it displays is based on the boolean value isBlue.

Let's say that isBlue is true by default. This component will have accessed isBlue and numberofBlueFish, meaning it needs to re-render if either of those values change. It does not need to re-render if numberOfRedFish changes because it is not even being used.

If isBlue is true, numberOfRedFish can change from 1 to 100000, and it just doesn't matter to this component, because we're only showing the value of numberOfBlueFish.

Now let's say that isBlue is changed to false. The component re-renders because it is listening to isBlue, but this time it needs isBlue and numberOfRedFish. It does not need to re-render when numberOfBlueFish updates, because it is not even using that value.

If isBlue is false, numberOfBlueFish can change from 1 to 100000, and it just doesn't matter to this component, because we're only showing the value of numberOfRedFish.

For this reason, all subscriptions to the global state are removed every render cycle, because they may not be used anymore. The newly-used global state properties will be re-subscribed during the next render cycle, as they are accessed.

My gut feeling is telling me it's some kind of race condition in React, where the component is unsubscribing after render.

render is called -> subscriptions made -> render is complete
global state changes -> subscriptions dispatched
UNSUBSCRIBE SHOULD BE CALLED HERE
re-render is called -> subscriptions made -> re-render is complete

If for some reason, the UNSUBSCRIBE SHOULD BE CALLED HERE is happening at a different location, like perhaps after re-render is complete, that would account for this. This may be the case if the cleanup step occurs asynchronously while the re-render occurs synchronously.

const cleanup = async () => {
  await new Promise((resolve) => {
    alert('unsubscribe');
    resolve();
  });
};

const render = () => { /* ... */ };

cleanup();
render();

In the above example, where cleanup is async and render is synchronous, you'll find that alert('unsubscribe'); occurs after render has finished executing.

If this is the case, which I don't know for sure, this almost seems like a bug with React, but it may be by design. I know that useEffect is considered somewhat asynchronous in that it fires after the DOM has been updated. There is a useLayoutEffect which occurs "more" synchronously in that it fires before the DOM has been updated and allows for state changes that trigger multiple re-renders within a single render cycle.

My next idea would be to change useEffect here to useLayoutEffect to see if the cleanup step becomes synchronous, executing earlier in the lifecycle, and cleaning up before re-render instead of after the new subscriptions are made.

[m4ttheweric]

Thanks for the detailed explanation of the thought behind your design, I think I get it for the most part.

And, I read the other ticket before creating this one, where you recommended useLayoutEffect for a similar issue. I actually did try changing it to useLayoutEffect in the source code, but it didn't resolve it for me. Just for sanity, I tried it again now, and still no luck.

[m4ttheweric]

So, in the property specific one, you subscribe and cleanup all in the same effect. This is how we are all taught to use subscriptions in components in the hooks world:

useEffect((): VoidFunction => {
    globalStateManager.addPropertyListener(property, forceUpdate);
    return removeForceUpdateListener;
  });

I wonder if you can refactor somehow to put your global listener in the same effect as the cleanup, I bet this would resolve. I am betting the issue is trying to use useEffect to clean something up without also having that corresponding subscription in the same effect. Since useEffect is not synchronous, I would not think it's reliable to cleanup one place and subscribe in another.

That being said, one thing I don't fully understand from the source code: Where is the subscription to the global state being done in this? I can see it easily in the code snippet above from the property specific stuff. But I can't quite see it in the global version.

[quisido]

Where is the subscription to the global state being done in this? I can see it easily in the code snippet above from the property specific stuff. But I can't quite see it in the global version.

The global state object is not:

{
  property: 'value',
  property2: 'value2',
}

It is:

{
  property: {
    get() {
      subscribeToChanges(componentInstance, 'property', forceUpdate); // <-- here
      return 'value';
    },
  },
  property2: {
    get() {
      subscribeToChanges(componentInstance, 'property2', forceUpdate); // <-- here
      return 'value2';
    },
  },
}

This is the "magic" of ReactN that automates subscriptions as if it were baked into React itself, without the boilerplate of having to specify to which properties you want to subscribe. Subscriptions are created for each property as it is accessed.

The code looks something like globalStateManager.spyState(forceUpdate), meaning "Give me the global state, but spy on each property, such that when that property is accessed, add the forceUpdate function as a callback for when that property changes." It could probably use a more descriptive name, but it does a mouthful!

In the case of useGlobal('property'), the subscription is added right then and there, when the hook is called.

In the case of useGlobal(), the subscription is not added until you access a property on the global object: global.property. This usually occurs synchronously (<div>{global.numberOfRedFish}</div>), but can occur asynchronously (<button onClick={() => { alert(global.numberOfBlueFish); }} />).

You are most likely right though, that the unsubscribe function is being called after the new subscriptions are made. This is accurately portrayed by how you noticed that the first re-render works fine (before unsubscription ever occurs) and only after unsubscribing does it stop working.

[quisido]

I've finally got around to investigating this today. I've added a unit test to reproduce this issue.

The code for useGlobal('property') 😅:

  useEffect(
    (): VoidFunction => {
      // We add the listener as an effect, so that there are not race conditions
      //   between subscribing and unsubscribing.
      // Subscribing outside of useEffect via `spyState()[property]` will
      //   cause the re-render subscription to occur before the unmount
      //   unsubscription occurs. As a result, the unmount unsubscription
      //   removes the re-rendered subscription.
      globalStateManager.addPropertyListener(property, forceUpdate);

      // If this component ever updates or unmounts, remove the force update
      //   listener.
      return removeForceUpdateListener;
    },
  );

Such a large comment to point out the exact issue of useGlobal().

I'm going to go ahead and implement your fix to use [] as a dependency array, because I think it is more important that the component re-render when the state changes than it is important than it doesn't re-render when edge case extraneous state changes.

Thanks a ton for your investigations!

[m4ttheweric]

@CharlesStover Hey, I am just glad I was able to help contribute even a tiny amount to a library I have really enjoyed using. Thanks for implementing the fix, and yay for collaboration and many more successful re-renders in the future : )

[de1mat]

Enjoyed reading this ticket and seeing the collaboration. Thanks for your efforts 👌