Inconsistent behavior based on the order that components are rendered (React)

[HBake]

With nested molecules and multiple ScopeProviders, the order that components are rendered changes the behavior in a strange way.

Reproduction on CodeSandbox

https://codesandbox.io/p/sandbox/focused-oskar-9z7ytd?file=%2FApp.tsx

Setup

const NameScope = createScope<string>("eve");
const NameMolecule = molecule((_mol, scope) => {
  const name = scope(NameScope);
  return atom(name);
});

const CombinedMolecule = molecule((mol) => {
  const nameAtom = mol(NameMolecule);
  return nameAtom;
});

function BaseCounter({ nameAtom }: { nameAtom: PrimitiveAtom<string> }) {
  const [name, setName] = useAtom(nameAtom);

  return (
    <div>
      <p>({name})</p>
      <button onClick={() => setName((c) => c + 1)}>Update Name</button>
    </div>
  );
}

function Counter1() {
  const nameAtom = useMolecule(CombinedMolecule);
  return <BaseCounter nameAtom={nameAtom} />;
}

function Counter2() {
  const nameAtom = useMolecule(NameMolecule);
  return <BaseCounter nameAtom={nameAtom} />;
}

Bug

The first 2 names are synced together. The third and fourth names are not synced to anything.

{/******* Uncommenting this would cause the names to by synced  *********/}

{/* <ScopeProvider scope={NameScope} value="bob">
    <Counter1 />
</ScopeProvider> */}

<ScopeProvider scope={NameScope} value="bob">
    <Counter2 />
</ScopeProvider>
<ScopeProvider scope={NameScope} value="bob">
    <Counter2 />
</ScopeProvider>
<ScopeProvider scope={NameScope} value="bob">
    <Counter1 />
</ScopeProvider>
<ScopeProvider scope={NameScope} value="bob">
    <Counter1 />
</ScopeProvider>

Notes

• If the default value for NameScope is "bob", the names are all synced
• If you use one scope provider, the names are all synced

[loganvolkers]

Thanks for the complete and detailed report, this is a nicely put together sandbox. The problem is a different atom instance gets created, so it's a different state value.

You can further simplify the reproduction by replacing an atom with just a random number, for example: https://codesandbox.io/p/sandbox/focused-oskar-9z7ytd?file=%2FApp.tsx

Looks like I need to add a test case to ScopeProvider.test.tsx for this particular rendering case.

This should probably be tested across both StrictMode and non-strict mode. It may be a bug in the core library, or may just affect React.

[HBake]

@loganvolkers Thanks for the speedy reply!

It seems that you linked back to the original CodeSandbox that I posted, but yeah it can be reproduced without atoms.

It is easy to enable/disable ScrictMode in the CodeSandbox in index.tsx. FWIW the issue happens in both cases in my tests.

[loganvolkers]

Reproduced via test cases in #66 across both strict mode and non-strict mode.

[loganvolkers]

The reason for this relates to how molecule values get created and possibly discarded. This behaviour was added to comply with React strict mode. Injectors will create values, but won't persist or share them until a subscription is started.

This means that during initial render, you might have 2 different values for a base molecule, with one being discarded based on when useEffect is called (or not).

Later on during the subscription lifecycle (or component lifecycle in React) then a molecule will be "mounted", which will save it's value to the cache. That is, unless there's already a value in the cache, in which case the value will be discarded and we'll just grab the value from the cache.

This is what happens when we start a subscription for a derived molecule first. It will store the base value in the cache, and then other uses of the base molecule will be discarded and replaced with the cache value.

The problem for this case is that if we start the subscription for the base molecule first, then when the derived molecule is used, it checks if it's the right value, but it never checks if it's dependencies are still up-to-date. So it never gets discarded.

To fix this, we need to make sure that when a derived molecule is used, it always checks that it has the latest value of all of it's dependencies. If it isn't up to date, then the derived molecule should be re-run.

I have a fix for this in the issue-64 branch, but I don't fully trust that my solution is robust yet. I need to add more test cases and assertions before I am confident that this covers all possible dependency use cases.

[rbalicki2]

Just saw this linked to on Twitter.

Y'all may be interested in looking at https://github.com/isographlabs/isograph/blob/main/libs/isograph-react-disposable-state/src/useCachedPrecommitValue.ts, which allows you to execute some function once during the initial render of a component, and reuse that value going forward. (That is purposefully a vague statement, as the library is quite generic. So, that function could be "create a base molecule" or "start a subscription".)

This can (probably) solve the issue articulated here.

[loganvolkers]

Thanks for noticing and commenting @rbalicki2 -- it looks like you're solving a similar problem!

Bunshi has two internal injector methods:

• useLazily that returns a value a subscription to start and stop. Calling start may return a different value, which seems similar to getOrPopulateAndTemporaryRetain in ParentCache.
• use that internally uses useLazily and immediately calls start.

To ensure no memory leaks, anything that calls start must have a follow up with stop.

Since you're the author of @isograph/react-disposable-state could you help me out by providing an example of how this would work?

I had started building a similar timeout-based eviction policy into Bunshi V2.1 but I ran into problems getting the test suite to pass with the subtle change in behaviour.

[rbalicki2]

Yeah, happy to dive into more detail! Do you want to hop on a call at some point to discuss? Happy to run you through this. The easiest way to coordinate that would be to DM me @statisticsftw on Twitter or email me ([email protected]).

Anyway, I just wrote up an extremely detailed description of how useLazyReference, which makes a network request during the initial render of a component, works. It uses useCachedPrecommitValue under the hood.

Here it is.

[loganvolkers]

Thanks for the write-up @rbalicki2 and the offer. I may just hit you up on it on your help via a call.

In theory the approach in useLazyReference could be applied to Bunshi. In some ways that would be ideal, because we would have the same values temporarily retained across the two components.

That said, it seems like there would still be the possibility of no temporarily retained value being present, in the case where components take too long. In this case, we would still need to handle our graph invalidation case.

Given this situation, we may still need to do a deep refresh (like in #66) as a safety measure.

I'm going to keep pulling on the approach in #66 to solve this front and center bug in Bunshi.

The Future

I've created a new discussion around temporary retain in #68 to tackle some of the possibilities there.

[HBake]

Hi Logan, is there any update on this? I would like to add scoping to ≈100 atoms that are currently global.

I am very happy with bunshi generally, and I would like to use it for this purpose as well, but it would be a complex nested molecule structure. This bug would inevitably cause many headaches.