jotaijs / jotai-scope

MIT License
55 stars 4 forks source link

atom derived from a scoped atom can't read it's own value #24

Closed il3ven closed 3 months ago

il3ven commented 6 months ago

Code Sandbox Link


Code

const countAtom = atom(0);
const countMultiplierAtom = atom(
  (get) => get(countAtom) * 2,
  (get, set) => {
    console.log(get(countMultiplierAtom));
  }
);

function Counter() {
  const [count, setCount] = useAtom(countAtom);
  return (
    <div>
      <div>Count: {count}</div>
      <button onClick={() => setCount(count + 1)}>Increment</button>
    </div>
  );
}

function DoubleCounter() {
  const [count, logCount] = useAtom(countMultiplierAtom);
  return (
    <div>
      Double Count: {count}
      <button onClick={() => logCount()}>Log Count</button>
    </div>
  );
}

export default function App() {
  return (
    <div className="App">
      <Counter />
      <DoubleCounter />
      <br />
      <br />
      <div style={{ border: "1px solid black" }}>
        <span>Scoped</span>
        <ScopeProvider atoms={[countAtom]}>
          <Counter />
          <DoubleCounter />
        </ScopeProvider>
      </div>
    </div>
  );
}

Steps to Reproduce

Video Demo

https://github.com/jotaijs/jotai-scope/assets/4337699/a0beebcd-3d33-4894-9ebd-cbce667be3d1

yf-yang commented 6 months ago

This is a challenging situation.

By design, a derived atom does not know whether it is scoped. When it subscribes to another atom (for example, Atom A), the ScopeProvider will navigate to the appropriate atom and establish their connection. This way, when Atom A changes, regardless of where, the derived atom will be notified.

Since the connection is implemented as a listener callback on Atom A, there is no way to determine from within the derived atom itself whether it is scoped.

yf-yang commented 6 months ago

As a temporary solution, would you mind rewrite the read function get(countAtom) * 2? I am not sure if the read function is computationally heavy or the dependency chain is very long in your actual case. I need to think it over.

yf-yang commented 6 months ago

Would you like to describe your use case 👀 ? In what scenario would you get a derived atom's value in its write function?

@dai-shi Do you think we should support the scenario? We have an assumption that only primitive atom would get itself. For derived atoms, it is not possible to get itself from its read function, but I do not realize it is possible in its write function.

Consider the following case, in logDoubledCount and logAnotherDoubledCount, we can't tell whether the atom is scoped or not.

To achieve the goal, we have to add a property to the intercepted copy to mark it as scoped, and if it does not exist, search through the scopes to figure out whether it is scoped.

Conceptually that would be problematic, because at least one read call needs to be executed, then we can figure out if it is scoped. If useSetAtom is called before the useAtomValue is called, then we still do not know if an atom is scoped or not.

const countAtom = atom(0);
const doubledCountAtom = atom(
  (get) => get(countAtom) * 2,
  (get) => console.log(get(countAtom)),
);
const anotherCountAtom = atom(0);
const doubledAnotherCountAtom = atom(
  (get) => get(anotherCountAtom) * 2,
  (get) => console.log(get(doubledAnotherCountAtom)),
);

const Counter = () => {
  const [count, setCount] = useAtom(countAtom);
  const [doubledCount, logDoubledCount] = useAtom(doubledCountAtom);
  const [anotherCount, setAnotherCount] = useAtom(anotherCountAtom);
  const [doubledAnotherCount, logDoubledAnotherCount] = useAtom(
    doubledAnotherCountAtom,
  );
  return (
    <>
      <div>
        <span>
          count: {count} (doubled: {doubledCount})
        </span>
        <button
          type="button"
          onClick={() => {
            setCount((c) => c + 1);
            logDoubledCount();
          }}
        >
          increment
        </button>
      </div>
      <div>
        <span>
          another count: {anotherCount} (doubled: {doubledAnotherCount})
        </span>
        <button
          type="button"
          onClick={() => {
            setAnotherCount((c) => c + 1);
            logDoubledAnotherCount();
          }}
        >
          increment
        </button>
      </div>
    </>
  );
};

const App = () => {
  return (
    <ScopeProvider atoms={[countAtom]}>
      <Counter />
    </ScopeProvider>
  );
};
yf-yang commented 6 months ago

OK, I come up with a solution: If that is really needed, you can explicitly mark your derived atom as scoped

<ScopeProvider atoms={[countAtom, countMultiplierAtom]} />

https://codesandbox.io/p/sandbox/jotai-forked-kd8yxs

I do not intend to fix the case. I also suspect if the case is common enough and should be documented. Waiting for Daishi's opinion.

dai-shi commented 6 months ago

wow, that sounds tricky. (i'm not 100% following, but...)

If that is really needed, you can explicitly mark your derived atom as scoped

...it sounds like a reasonable workaround. Yes, the use case is valid, so it should be documented.

il3ven commented 6 months ago

If that is really needed, you can explicitly mark your derived atom as scoped

@yf-yang Thank you for looking into this. However, for my use case I can't mark the derived atom as scoped because I am using atomFamily.

Here's a better example for my use case.

https://github.com/jotaijs/jotai-scope/assets/4337699/3454320b-6ae9-4de4-a6a1-0d3ff9749827

CodeSanbox Link

In the video, the un-scoped variant works fine. Clicking "Set Count to 2" sets the count atom to 2 as expected. For the scoped variant, clicking "Set Count to 0" should do no change but it sets the scoped count atom to 4 because it is using the un-scoped value.

yf-yang commented 6 months ago

What about refactor the code like this?

const multiplierAtom = getCountMultiplierAtom(2);

function Counter() {}}

function App() {}

https://codesandbox.io/p/sandbox/jotai-atomfamily-forked-69sh5d

I think when using atom, we need to guarantee its lifetime is not shorter than the component which is using it. Therefore, normally we need to declare it globally or wrap it with useMemo. atomFamily is an outlier, as it is actually maintaining an internal invisible global scoped, app lifelong map that stores key-atom connection.

So, what I mean are three points:

  1. Initiating an atom in a component without useMemo is weird, but atomFamily "accidentally" can work with the pattern. Similarly, we can always put the atomFamily call outside.
  2. Since atomFamily is global, this call also works: <ScopeProvider atoms={[countAtom, getCountMultiplierAtom(2)]} />. https://codesandbox.io/p/sandbox/jotai-atomfamily-forked-z8wvhz
  3. If the value you passing to getCountMultiplierAtom is dynamically changing (I guess that's the case you have to do so), then I think you need to maintain which values you are using, then make them all scoped would work. I offer this solution because you need to take care the lifecycles of those dynamic atoms created by atomFamily, when they are abandoned, you have to release the atom, so you have to maintain an array of used keys anyway.

The third solution would be something like that:


// each multiplier's count
const usedMultipliersAtom = atom({});

function useMultiplierAtom(multiplier) {
  const [usedMultipliers, setUsedMultipliers] = useAtom(usedMultipliersAtom);
  useEffect(() => {
    if (!Object.prototype.hasOwnProperty.call(multiplier)) {
      setUsedMultipliers(prev => { ...prev,  [multiplier]: 0 });
    }
    setUsedMultipliers(({ [multiplier]: prevValue, ...other }) => ({ ...other, [multiplier]: prevValue + 1 }));

    return () => {
      setUsedMultipliers(({ [multiplier]: prevValue, ...other }) => ({ ...other, [multiplier]: prevValue - 1 }));
      if (usedMultipliers[multiplier] === 1) {
        getCountMultiplierAtom.remove(multiplier);
      }
    }
  }, [multiplier]);
  return getCountMultiplier(multiplier);
}

// Then, in your app
<ScopeProvider atoms={[
  countAtom, 
  ...Object.keys(usedMultipliers).map(multiplier => getCountMultiplierAtom(multiplier))
]} />
dmaskasky commented 4 months ago

at least one read call needs to be executed

Would this be true with:

const countMultiplierAtom = atom(
  (get) => get(countAtom) * 2,
  (get, set) => {
    get(countMultiplierAtom)
  }
)

Perhaps for every atom, store.get should check the atomSet up the scope chain.