jotaijs / jotai-scope

MIT License
55 stars 4 forks source link

Nested Scope Consistency Question #32

Closed dmaskasky closed 3 months ago

dmaskasky commented 4 months ago

I'm wondering about the current implementation for nested scope. Is it strange that derivedAtom1 does NOT use the baseAtom scoped to layer2?

<ScopeProvider atoms={[derivedAtom1]}>
  <Counter counterClass="layer1" />
  <h2>Layer2: Base and derived2 are scoped</h2>
  <p>
    derived1 should use layer1&apos;s atom, base and derived2 are layer 2
    scoped
  </p>
  <ScopeProvider atoms={[baseAtom, derivedAtom2]}>
    <Counter counterClass="layer2" />
  </ScopeProvider>
</ScopeProvider>

For the single layer case, derivedAtom1 would use baseAtom inside Counter below, instead of the global baseAtom.

<ScopeProvider atoms={[baseAtom, derivedAtom2]}>
    <Counter counterClass="layer1" />
</ScopeProvider>

I think it would be more consistent if derivedAtom1 uses layer2 baseAtom in layer2 Counter.

Do you see any pitfalls with this proposed behavior? Are there any advantages in keeping the current behavior?

yf-yang commented 4 months ago

Go back to this case: https://github.com/jotaijs/jotai-scope/issues/28#issuecomment-2094682317

So, when derived1 is accessed in layer2, there are two possibilities:

  1. derived1 is scoped in a parent scope.
  2. derived1's dependency is scoped in current scope.

There is no conflict between those two possibilities, so they can both happen. Therefore, we need to define the priority.

In current implementation, we check derived1 itself in parent scope first, then check its dependencies.

I'm not sure if we can offer a better rule. For example,

<ScopeProvider atoms={[derivedAtom1]}>
  <Counter counterClass="layer1" />
  <ScopeProvider atoms={[baseAtom, derivedAtom2]}>
    <Counter counterClass="layer2" />
  </ScopeProvider>
</ScopeProvider>

You think in layer 2 derived1 should access nested one? Then what about this case?

<ScopeProvider atoms={[derivedAtom1]}>
  <Counter counterClass="layer1" />
  <ScopeProvider atoms={[derivedAtom2]}>
    <Counter counterClass="layer2" />
  </ScopeProvider>
</ScopeProvider>

This case is actually part of the previous mentioned comment. I wonder if you want some rule like "if the dependency is explicitly scoped, then it has higher priority".

dmaskasky commented 4 months ago

Case Study

Consider these two cases. How are they similar? How are they different?

const baseAtom = atom(0)
const derivedAtom1 = atom((get) => get(baseAtom))

function Counter() {
  const [count, setCount] = useAtom(derivedAtom1)
  ...
}

// Case 1
<Provider>
  <Counter counterClass="layer1" />
  <ScopeProvider atoms={[baseAtom]}>
    <Counter counterClass="layer2" />
  </ScopeProvider>
</Provider>

// Case 2
<ScopeProvider atoms={[derivedAtom1]}>
  <Counter counterClass="layer1" />
  <ScopeProvider atoms={[baseAtom]}>
    <Counter counterClass="layer2" />
  </ScopeProvider>
</ScopeProvider>

How are they similar?

For both Case 1 and Case 2, derivedAtom1 is scoped to the parent scope of layer2. It is not scoped to layer2.

How are they different?

In Case 1, derivedAtom1 is just an unscoped (eg global) derived atom from the store given by the top-level provider. In layer2 it gets it's value from the scoped baseAtom.

In Case 2, derivedAtom1 is in the parent scope. In layer2, current implementation has it get it's value from layer1 despite baseAtom being scoped to layer2. This is different behavior from Case 1.

Summary

With existing implementation we can make the following statement: " including atoms in ScopeProvider somehow makes them special because their atom dependencies' scope is fixed to the scope of the atom even if those dependencies are explicitly scoped in nested ScopeProviders. "

My thoughts on this

We should not make that statement in the Summary above. It artificially restricts usage and goes against purpose of using ScopeProvider. And currently, there is no workaround for this specialness property of scoped derived atoms.

The ScopeProvider lets applications mix global atoms with scoped ones, but fixing the scope of atom dependencies of scoped atoms prevents mixing scopes of atom dependencies for nested scopes.

  1. If derivedAtom1 is scoped in level1 and its dependency dep1 is scoped in level2, to use scoped dep1 in level2 derivedAtom1 must also be scoped in level2.
  2. And if derivedAtom1 has another level1 scoped dependency dep2, it must make dep2 a level2 scope as well.

For nested scopes, this makes derived atoms all-or-nothing, and the benefits of using ScopedProvider are lost.

Proposal

The priority should be: The closest scoped atom is the winner.

getAtom Algo

const derived1 = atom((get) => get(dep1), (get, set) => set(dep1, get(dep1) + 1))

To get dep1:

  1. if dep1 is explicitly added to current scope, return it
  2. if derived1 is explicitly added to current scope, copy dep1 to current scope map if DNE and return it
  3. recurse up scope ancestor path repeating (1) and (2)

To get derived1:

  1. if dep1 is explicitly added to current scope, return it
  2. recurse up scope ancestor path repeating (1)

What about the case you mentioned?

You think in layer 2 derived1 should access nested one? Then what about this case?

<ScopeProvider atoms={[derivedAtom1]}>
  <Counter counterClass="layer1" />
  <ScopeProvider atoms={[derivedAtom2]}>
    <Counter counterClass="layer2" />
  </ScopeProvider>
</ScopeProvider>

For this case, in layer2

  1. derivedAtom1 accesses baseAtom scoped to layer1
  2. derivedAtom2 accesses baseAtom scoped to layer2

Related:

yf-yang commented 4 months ago

OK, so the resolution direction is

  1. Resolve dependencies
  2. Resolve to ancestor scopes.

I'm still unclear what's the boundary of 1, when we will consider 1 as finishes and start to try 2?

Let's dive deeper into details. Suppose some of atomA's dependencies are scoped, then atomA is scoped, so we finishes in 1. If that's not the case, should we check the dependencies' ancestor scope atom first, or we do not check anything and directly check atomA in parent scope?

That's the conceptual question. Consider those two cases. You can ignore derived2. Which base would derived1 in layer3 access?

<ScopeProvider atoms={[derived1]}> // layer1
  <ScopeProvider atoms={[base]}> // layer2
    <ScopeProvider atoms={[]}> // layer3
      <Counter />
    </ScopeProvider>
  </ScopeProvider>
</ScopeProvider>
<ScopeProvider atoms={[base]}> // layer1
  <ScopeProvider atoms={[derived1]}> // layer2
    <ScopeProvider atoms={[]}> // layer3
      <Counter />
    </ScopeProvider>
  </ScopeProvider>
</ScopeProvider>
dmaskasky commented 4 months ago

Language convention: nested scope is colloquially greater than parent scope (ie layer2 > layer1).

Q: should we check the dependencies' ancestor scope atom first? A: derived1 scope > dep1 scope ? derived1 : dep1.

Example 1

<ScopeProvider atoms={[derived1]}> // layer1
  <ScopeProvider atoms={[base]}> // layer2
    <ScopeProvider atoms={[]}> // layer3
      <Counter />
    </ScopeProvider>
  </ScopeProvider>
</ScopeProvider>

Q: Which base would derived1 in layer3 access? A: layer2. Because base layer2 > derived1 layer1.

Example 2

<ScopeProvider atoms={[base]}> // layer1
  <ScopeProvider atoms={[derived1]}> // layer2
    <ScopeProvider atoms={[]}> // layer3
      <Counter />
    </ScopeProvider>
  </ScopeProvider>
</ScopeProvider>

Q: Which base would derived1 in layer3 access? A: layer2. Because derived1 layer2 > base layer1.

yf-yang commented 4 months ago

OK, so the rule would be: Resolve the dependency in current scope. Then, resolve it in the ancestor scope, recursively, until some atoms of the dependencies is scoped.

Conceptually, it makes sense, but when it comes to implementation, since dependency graph is dynamically built, we need to traverse the whole graph to know if any of the dependency is scoped, then go upward to the ancestor scope.

In worst case, if a derived atom and its base atom is global, if it is deeply nested, then the computational complexity would be O(Number of nest scopes * Number of atoms in the dependency).

yf-yang commented 4 months ago

base -> derived1 -> derived2 Example 1:

<ScopeProvider atoms={[derived1]}> // layer1
  <ScopeProvider atoms={[base]}> // layer2
    <ScopeProvider atoms={[]}> // layer3
      <Counter />
    </ScopeProvider>
  </ScopeProvider>
</ScopeProvider>

When calling useAtomValue(derived2):

Example 2:

<ScopeProvider atoms={[]}> // layer1
  <ScopeProvider atoms={[]}> // layer2
    <ScopeProvider atoms={[]}> // layer3
      <Counter />
    </ScopeProvider>
  </ScopeProvider>
</ScopeProvider>

When calling useAtomValue(derived2):

yf-yang commented 4 months ago

Horizontal line: atom.read/atom.write Vertical line: callback handled by ScopeProvider context internally

Example1 Original Implementation (Wrong)

image

Current Implementation

image

Proposed Implementation

image
yf-yang commented 4 months ago

@dai-shi Do you have any comments on different implementations? I have three concerns:

  1. atom.read/atom.write are handled by users, they could be expensive (are they?), they could have side effects (do they?)
  2. I am still not sure if it is possible to return an additional value indicating if the dependency atom is scoped to the derived atom.
  3. Write is more difficult than read. Write should have exactly one side effect at the end. I don't know how to do it gracefully.
yf-yang commented 4 months ago

@dmaskasky After checking the code, I think it is impossible to tell derived atom from base atom that base atom is scoped or not. The only stuff that can be passed to derived atom is the base atom's state value.

dmaskasky commented 4 months ago

Example 1:

<ScopeProvider atoms={[derived1]}> // layer1
  <ScopeProvider atoms={[base]}> // layer2
    <ScopeProvider atoms={[]}> // layer3
      <Counter />
    </ScopeProvider>
  </ScopeProvider>
</ScopeProvider>

layer3: traverse derived2 -> derived1 -> base

this should use base (layer2) because in layer3, derived1 -> base (layer2).

Example 2:

<ScopeProvider atoms={[]}> // layer1
  <ScopeProvider atoms={[]}> // layer2
    <ScopeProvider atoms={[]}> // layer3
      <Counter />
    </ScopeProvider>
  </ScopeProvider>
</ScopeProvider>

result is invalid

what does invalid mean? since none are scoped, it should use the closest Provider store otherwise default global store.

Needs 3 * (3 + 1) atom.read call

I don't think atom.read should get called on every level. Call once, then look up the chain for the atom dependency in the weakSet.

Also, I looked at implementation and it looks like it is copying the atom at every level. I think it would be more performant to copy atom config only for atoms in the atomSet and its dependencies.

After checking the code

I think the code will need to be changed to allow for atoms of parent scopes to "lexical scope" their nested scopes. I recommend a data structure to make this easier to accomplish while staying in the current scope.

dai-shi commented 4 months ago

atom.read/atom.write are handled by users, they could be expensive (are they?), they could have side effects (do they?)

write can have side effects. by contract, read should be idempotent, but as optimization, it's not invoked unnecessarily. both can be expensive technically.

dmaskasky commented 4 months ago

yes, read should be called only once.

dmaskasky commented 4 months ago

I am still not sure if it is possible to return an additional value indicating if the dependency atom is scoped to the derived atom.

is this necessary? I think an atom's membership to a weakSet should be all that is needed to indicate scope. Derived atom dependencies are implicitly scoped.

Write should have exactly one side effect at the end.

Write can have any number of side-effects before and after zero to many calls to get/set, and a value may be returned from write.

dai-shi commented 4 months ago

yes, read should be called only once.

Though, it's not a guaranteed feature for the future. Our mental model should be "read" is no side effect.

dmaskasky commented 4 months ago

Yes, for instance atomEffect does not assume read is called only once, but unnecessary calls should be avoided for performance. In this case, I don't think read needs to be called more than once since the mechanism for resolving atoms is built into the overrided getter and setter functions.

yf-yang commented 4 months ago

image

Let me try to explain in details how this one should be implemented.

At layer 3

  1. Call derived2.read(), it is a function that calls get(derived1).
  2. Then similarly, call derived1.read(), it calls get(base)
  3. Then, base.read() calls get(base) Now after all of them are called, we finally figure out that "all of derived2's dependencies are not scoped", so we go to layer2.

At layer 2

  1. Call derived2.read(), it is a function that calls get(derived1).
  2. Then similarly, call derived1.read(), it calls get(base)
  3. Since the get function is actually hooked by us, we now figure out that "base is scoped", so we stop here, access layer 2's atoms.

Now, the information that "no atom is scoped in derived2's dependency chain at layer 3" should be sent to derived2, but there is no easy way to achieve that.

yf-yang commented 4 months ago

Emmm... I've come up with some ugly hack.

When store.set is called,

  1. We enter a "dependency resolution stage". Init an empty Set.
  2. During the stage, we bypass original jotai mechanism. Instead, we recursively calls atom.read/atom.write, to find all the dependency atoms.
  3. Since we've collected all the dependency atoms, the first stage finishes.
  4. Then, for each dependency atoms, we find each one's deepest scope. Then, we find the deepest scope of all the atoms. That's the target scope.
  5. Then, use the atom in the target scope to actually call store.set

A bit drawback is that, dependency could vary based on different value at different scope. So, dependency resolution stage" should be called at every scope until we find the correct one. So the computational efficiency problem still exist.

yf-yang commented 4 months ago

The root problem: dependency is dynamically computed. The only way to figure out the actual dependency is calling atom.read/atom.write. So, each time a horizontal arrow of the graph is traversed, an atom.read/atom.write is called.

Be aware that dependency may not even be the same in different scope. DerivedAtom could access base1 or base2 given different value of base3. Only one dependency resolution is not enough.

dmaskasky commented 4 months ago

This is looking closer to what I'm imagining.

The only change I would make is to synchronously get the atom from the nearest scope first before calling its read fn. That way dynamic deps edge case is handled correctly. So for each dep recursive, always look starting at the nearest scope.

I believe only one read should suffice. 😌

dmaskasky commented 4 months ago

I might be wrong but I think the requisite change should also address https://github.com/jotaijs/jotai-scope/issues/24

dmaskasky commented 4 months ago

I'd be happy to review a PR if you have an idea on how to fix.

yf-yang commented 4 months ago

The only change I would make is to synchronously get the atom from the nearest scope first before calling its read fn.

We can't do it.

Consider this case:

DerivedAtom could access base1 or base2 given different value of base3.

We need base3's value to get the correct dependency atom of derived. We need to know derived and base3's scope to get the correct value of base3. We don't know base3's scope. The only way is to recursively iterate each scope, then figure out if the scope and the dependency can be simultaneously determines in the iterating scope.

yf-yang commented 4 months ago
const derived = atom(get => get(base3 > 0 ? base1 : base2));

<ScopeProvider atoms={[derived]}> // layer1
  <ScopeProvider atoms={[base3]}> // layer2
    <ScopeProvider atoms={[base2]}> // layer3
      <ScopeProvider atoms={[base1]}> // layer4
        <Counter />
      </ScopeProvider>
    </ScopeProvider>
  </ScopeProvider>
</ScopeProvider>
yf-yang commented 4 months ago

Emmm, maybe I can come up with another example that breaks the rule.

<ScopeProvider atoms={[derived]}> // layer1
    <ScopeProvider atoms={[atom1]}> // layer2
      <Counter />
    </ScopeProvider>
</ScopeProvider>

So from the rule, in layer2, derived should access atom1 of layer2. However, which atom2 will it access? global atom2 seems breaks the scope. layer1 atom2 introduces implicit scoping, I believe it would be unexpected in most cases.

dmaskasky commented 4 months ago

I'm not sure I follow. In your example above, base1 vs base2 vs base3 are different atoms - not atoms of different scopes.

derived1 can only declare that it needs the value from baseAtom. Remember atoms are just definitions and scopes are tied to the store and not the atom definition. derived1 would not be able to dynamically switch to a different baseAtom scope.

Even if it were possible, the strategy I have in mind would still be able to handle this case efficiently.

I might put something together when I find the time.

yf-yang commented 4 months ago

Hmmm, the same atom in different scopes are implemented as different atoms (that's aligned with the behavior that the same atom has different independent value in different scope).

Before we further discuss that, maybe we can first check this example https://github.com/jotaijs/jotai-scope/issues/32#issuecomment-2105654309. I realize this rule may be harder to understand. I need to figure out if this example makes sense before discussing implementations.

dmaskasky commented 4 months ago

Ok, I understand your concern.

We need base3's value to get the correct dependency atom of derived. We need to know derived and base3's scope to get the correct value of base3. We don't know base3's scope. The only way is to recursively iterate each scope, then figure out if the scope and the dependency can be simultaneously determines in the iterating scope.

const derived = atom(get => get(base3 > 0 ? base1 : base2));

<ScopeProvider atoms={[derived]}> // layer1
  <ScopeProvider atoms={[base3]}> // layer2
    <ScopeProvider atoms={[base2]}> // layer3
      <ScopeProvider atoms={[base1]}> // layer4
        <Counter />
      </ScopeProvider>
    </ScopeProvider>
  </ScopeProvider>
</ScopeProvider>

There's actually two ways to handle this.

  1. recurse each scope then figure out if the atom dependency has a scope and which is the closest scope. O(n) read, O(1) insert.
  2. maintain a flattened map of all scoped atoms at each scope. O(1) read, O(n) insert. All nearest scoped atoms in the current scope must be in the weakmap.

Both read and insert must be synchronous. Since insert is a one-time cost on the average case, we should optimize for read making (2) the better option.

yf-yang commented 3 months ago

We already maintain the weakmap as you said in 2. The key here is "we don't know which atoms are in the dependencies", so even we know which atoms are scoped, we don't know if the atom being used is scoped or not.

Shall we discuss https://github.com/jotaijs/jotai-scope/issues/32#issuecomment-2105654309 first?

dmaskasky commented 3 months ago

Yes. Another challenge is that if a derived atom subscribes to changes to a scoped atom. If that scoped atom gets removed from the atoms prop of the ScopeProvider, then the derived atom now points to the global atom, but isn't subscribed to it. I haven't checked if this is solved by router atom or not.

yf-yang commented 3 months ago

then the derived atom now points to the global atom, but isn't subscribed to it.

It is already taken into considerations.

https://github.com/jotaijs/jotai-scope/tree/main/examples/02_removeScope

To make it work, we need to dynamically recompute everything when useAtom is called.

dmaskasky commented 3 months ago

I'm working on a refactor to close this issue and also https://github.com/jotaijs/jotai-scope/issues/24.

Are you on Discord? It would be great if we could offline some of this conversation not relevant to this issue. I have some questions specific to the current implementation.

You can find me on the Poimandres server as dmaskasky (previously dmaskasky#3642).