jotaijs / jotai-scope

MIT License
55 stars 4 forks source link

customAtoms are not properly scoped #28

Closed dmaskasky closed 4 months ago

dmaskasky commented 4 months ago

Summary

Custom atoms are not scoped because their base atom is not scoped. Is there a workaround for this use-case?

function customAtom<T>(initialValue) {
  const valueAtom = atom<T>(initialValue)   // <-- valueAtom is not scoped
  return atom(
    (get) => get(valueAtom),
    (_get, set, update: T) => set(valueAtom, update)
  )
}

const someCustomAtom = customAtom(0)

function MyComponent() {
  return (
    <ScopeProvider atoms={[someCustomAtom]}>
      <Counter />
    </ScopeProvider>
  )
}

CodeSandbox Example

https://codesandbox.io/p/sandbox/inspiring-grothendieck-gcc2jt

Other issues

Not sure if this is related to https://github.com/jotaijs/jotai-scope/issues/24. I don't think it is.


Proposal πŸ’‘

https://github.com/jotaijs/jotai-scope/issues/28#issuecomment-2088738873

Scope should apply to dependency trees.

const baseAtom = atom('base')
const derivedAtom1 = atom(
  (get) => 'derived1 ' + get(baseAtom)
  (_get, set, value) => {
    // sets the baseAtom scoped to derivedAtom1
    set(baseAtom, value) 
  }
)
const derivedAtom2 = atom((get) => 'derived2 ' + get(baseAtom))

function Component() {
  useHydrate([[derivedAtom1, 'scoped']])

  // deriveAtom1 is scoped, its baseAtom is scoped
  useAtom(derivedAtom1) // 'derived1 scoped'

  // deriveAtom2 is NOT scoped, its baseAtom is NOT scoped
  useAtom(derivedAtom2) // 'derived2 base'

  // baseAtom is NOT directly scoped, baseAtom is NOT scoped
  useAtom(baseAtom)

  ...
}
function App() {
  return (
    <ScopeProvider atoms={[derivedAtom1]} />
      <Component />
    </ScopeProvider> 
  )
}

This change will make understanding what is scoped a lot easier.

Scoping Writable Atoms

const baseAtom = atom('base')
const derivedAtom = atom((get) => 'derived ' + get(baseAtom))
const writableAtom = atom(null, (get, set, update) => set(baseAtom, update))

function Component() {
  // deriveAtom1 is scoped, its baseAtom is scoped
  useAtomValue(derivedAtom1)

  // writableAtom is scoped, the baseAtom it writes to is scoped
  // this is the same baseAtom copy used by derivedAtom1
  useSetAtom(writableAtom)

  // baseAtom is NOT directly scoped, baseAtom is NOT scoped
  useAtom(baseAtom)

  ...
}
function App() {
  return (
    <ScopeProvider atoms={[derivedAtom1, writableAtom]} />
      <Component />
    </ScopeProvider> 
  )
}

Revisiting Scoping Base Atoms

const baseAtom = atom('base')
const anotherAtom = atom('another')
const derivedAtom = atom((get) => 'derived ' + [get(baseAtom1), get(baseAtom1)])

function Component() {
  // derivedAtom reads the scoped baseAtom and the NOT-scoped anotherAtom
  useAtom(derivedAtom) // 'derived base,another'

  ...
}
function App() {
  return (
    <ScopeProvider atoms={[baseAtom]} />
      <Component />
    </ScopeProvider> 
  )
}
yf-yang commented 4 months ago

valueAtom is a primitive atom, customAtom is a derived atom that depends on valueAtom, although valueAtom is wrapped inside the function scope, the dependency would not change.

So here is the problem: you scope the derived atom only, but its actual primitive atom is not scoped, so the actual state (which is from the primitive atom) is shared.

Going back to the fundamental assumption, scoping derived atom does not make any sense.

Maybe we can warn users about the scenario? I am not so sure if it is easy to implement.

dai-shi commented 4 months ago

scoping derived atom does not make any sense.

If our design choice is to prohibit scoping derived atoms, we might be able to warn it by checking the read function. const isPrimitiveAtom = (a) => a.read === atom().read;

Wait, it's much easier: const isPrimitiveAtom = (a) => 'init' in a; (but there are some exceptions.)


So, in that design principle, what we could do for atom creators is to expose base atoms:

function customAtom(initialValue) {
  const valueAtom = atom(initialValue)
  const derivedAtom = atom(
    (get) => get(valueAtom),
    (_get, set, update: T) => set(valueAtom, update)
  )
  derivedAtom.baseAtoms = [valueAtom]
  return derivedAtom
}
dai-shi commented 4 months ago

Sounds interesting. Is baseAtoms existing api or is this a thought about what we could do in the future?

Well, I actually don't think it's a very good idea. We should hide base atoms to avoid leaking anything. It's not an official API, but libs can extend atom config. So, it's a community convention.

dmaskasky commented 4 months ago

Wouldn't the store have access to the atom's dependencies? Could the store provide some api to allow derived atoms to scope?

yf-yang commented 4 months ago

No, dependency is dynamically wrote in the read function (by calling get(anotherAtom)), so we cannot know which atom an atom depends on until we call the function. In a word, dependency is a dynamic concept. They cannot be statically known.

dai-shi commented 4 months ago

Wouldn't the store have access to the atom's dependencies? Could the store provide some api to allow derived atoms to scope?

Yeah, I'm not sure how that would feel though. In general, we don't expose things, as you know.

I guess, for jotai-scope, the developer's intention is important, and can't be automated anyway.

dai-shi commented 4 months ago

dependency is dynamically

Yeah, that's the tough one.

yf-yang commented 4 months ago

If our design choice is to prohibit scoping derived atoms

Well, we don't prohibit it. Actually they work as expected. The derived atom is scoped, then each copy subscribes to the same non-scoped primitive atom, so they are copies of derived atom whose value are exactly the same all the time. Harmless and meaningless.

const isPrimitiveAtom = (a) => 'init' in a;

Maybe we can add a warning for those cases. Jotai is very flexible, so there could be many exceptions we cannot handle, but at least we can warn about most of them, maybe that's OK?

what we could do for atom creators is to expose base atoms

For custom functions, I think people could decide themselves to return the base atom, if they really need it. For those packages in jotai ecosystem, I have no idea. I personally think we don't need an additional API since jotai is already broadly used, people may have already implemented lots of similar custom utilities.

dai-shi commented 4 months ago

Maybe we can add a warning for those cases. Jotai is very flexible, so there could be many exceptions we cannot handle, but at least we can warn about most of them, maybe that's OK?

If you accept the usage for some exceptions, we should provide a way to disable the warning. That said, I think it's a good idea to warn the use of derived atoms, because 99% is a mistake.

dmaskasky commented 4 months ago

Without knowing internal implementation details of jotai-scope, I think its a little surprising that jotai-scope can't scope atom creators. They are the recommended pattern in jotai and I feel a lot more developers are going to fall into the same trap.

There are lots of utility atoms out there that use the atom creator pattern, jotai-effect is one example.

yf-yang commented 4 months ago

I agree atom creator pattern is very common.

Before we dive into implementations, I'd like to discuss what that should be like. Suppose we want to add a rule: if a derived atom is scoped, all of its dependencies should be scoped as well.

I've come up with a conceptual case, suppose the dependency is like:

Intuitively, the dependency will soon become very hard to understand in a project with complex dependency network. Chain dependency is much easier, atom creator pattern is the easiest, but we cannot easily distinguish atom creator from other dependency patterns.

dmaskasky commented 4 months ago

which baseAtom copy will we get when calling useAtom(derivedAtom2)?

I see how this could be considered an indeterminate result, but I think the more common expectation is that if an atom is scoped, then its dependencies are implicitly scoped. Therefore, derivedAtom2 would see the scoped baseAtom in the same scope as derivedAtom1.

Suppose both derivedAtom1 and derivedAtom2 are scoped, then when we call useAtom(derivedAtom1) and useAtom(derivedAtom2), are they accessing the same baseAtom or different copies?

Are they in the same scope? Then yes. Otherwise, no.

These scenarios seem like they could be confusing for developers. Is there an integration with Jotai devtools to help understand what scope an atom is in?

we cannot easily distinguish atom creator from other dependency patterns

We're free to write the conventions and api. Surely a solution to this problem is not impossible.

dmaskasky commented 4 months ago

How about if this was opt-in with some new api to explicitly declare that all dependent atoms can be scoped?

/**
 * Makes the target derived atom and its dependencies able to be scoped by scoping just the target derived atom.
 */
declare function withScopable<T>(atom: Atom<T>): Atom<T>;

From a technical perspective, this could just set a boolean property on the atom that the store is designed to read.

yf-yang commented 4 months ago

Therefore, derivedAtom2 would see the scoped baseAtom in the same scope as derivedAtom1.

That sounds reasonable and worth trying. However, when I think of the implementation, here is a more practical problem:

Setting:

As a result,

Now in the last case, we find that derivedAtom2 and derivedAtom1 are accessing different baseAtom, which is not desired at all. The last case is really hard, because when we call useAtom(derivedAtom2), we will never know what the developer would call later, or they don't call anything. The only workaround I can think of is queueing another micro task to access the correct baseAtom, but React is synchronous (so do other frameworks).

We need to figure out if those two problems can be solved gracefully before proceeding to withScopable

dmaskasky commented 4 months ago

This whole edge case seems like something that could be resolved in the docs. Developers would be aware of this forking problem if the docs made mention explicitly prohibiting scoping derived atoms outside of the atom creator pattern.

Today, the docs don't even clearly state that base atoms aren't implicitly scoped when their derived atoms are scoped.

One final thing to note, to a laymen, creator atoms (such as atomWithQuery, atomEffect) are just special atoms. They shouldn't be concerned with their implementation when deciding whether they can scope atoms.

The utility I have proposed is really only intended to be used by library authors, who would understand this limitation. So I don't really consider it a problem.

Furthermore, I think Typescript should enforce only scopable atoms and base atoms to be allowed to be scoped. Non-scopable derived atoms should be prohibited.

dmaskasky commented 4 months ago

useAtom(derivedAtom1) and useAtom(derivedAtom2) can occur in any order

This order dependence seems like an implementation detail. If derivedAtom1 is scoped, then baseAtom is scoped. This is regardless of whether useAtom(derivedAtom1) is called first, useAtom(derivedAtom2) is called first, or useAtom(derivedAtom1) is not ever called. In all cases, the ancestor ScopeProvider has implicitly declared that BaseAtom is scoped. Also, consider ComponentA, ComponentB under ScopeProvider, ComponentA may have order [derivedAtom1, derivedAtom2] and ComponentB may have [derivedAtom2 derivedAtom1]. Order should not determine scope.

Moreover derivedAtom1 can conditionally depend on baseAtom. Without running derivedAtom1, jotai-scope is unable to determine whether baseAtom1 is a dependency or not.

New Idea πŸ’‘

Thinking more on this, I would like to amend my previous ask. I now think scope should apply to dependency trees for the case of derived atoms. The withScopable is no longer necessary.

const baseAtom = atom('base')
const derivedAtom1 = atom((get) => 'derived1 ' + get(baseAtom))
const derivedAtom2 = atom((get) => 'derived2 ' + get(baseAtom))

function Component() {
  // assuming useHydrate store context comes from the ScopeProvider
  useHydrate([[baseAtom, 'scoped']])

  // deriveAtom1 is scoped, its baseAtom is scoped
  useAtom(derivedAtom1) // 'derived1 scoped'

  // deriveAtom2 is NOT scoped, its baseAtom is NOT scoped
  useAtom(derivedAtom2) // 'derived2 base'

  // baseAtom is NOT directly scoped, baseAtom is NOT scoped
  useAtom(baseAtom)

  ...
}
function App() {
  return (
    <ScopeProvider atoms={[derivedAtom1]} />
      <Component />
    </ScopeProvider> 
  )
}

This change will make understanding what is scoped a lot easier.

dmaskasky commented 4 months ago

Scoping Writable Atoms

const baseAtom = atom('base')
const derivedAtom = atom((get) => 'derived ' + get(baseAtom))
const writableAtom = atom(null, (get, set, update) => set(baseAtom, update))

function Component() {
  // deriveAtom1 is scoped, its baseAtom is scoped
  useAtomValue(derivedAtom1)

  // writableAtom is scoped, the baseAtom it writes to is scoped
  // this is the same baseAtom copy used by derivedAtom1
  useSetAtom(writableAtom)

  // baseAtom is NOT directly scoped, baseAtom is NOT scoped
  useAtom(baseAtom)

  ...
}
function App() {
  return (
    <ScopeProvider atoms={[derivedAtom1, writableAtom]} />
      <Component />
    </ScopeProvider> 
  )
}
dmaskasky commented 4 months ago

Revisiting Scoping Base Atoms

const baseAtom = atom('base')
const anotherAtom = atom('another')
const derivedAtom = atom((get) => 'derived ' + [get(baseAtom1), get(baseAtom1)])

function Component() {
  // derivedAtom reads the scoped baseAtom and the NOT-scoped anotherAtom
  useAtom(derivedAtom) // 'derived base,another'

  ...
}
function App() {
  return (
    <ScopeProvider atoms={[baseAtom]} />
      <Component />
    </ScopeProvider> 
  )
}
yf-yang commented 4 months ago

This order dependence seems like an implementation detail. If derivedAtom1 is scoped, then baseAtom is scoped. This is regardless of whether useAtom(derivedAtom1) is called first, useAtom(derivedAtom2) is called first, or useAtom(derivedAtom1) is not ever called.

I know what you want, but unfortunately, the dependency computation happens when you call useAtom, not ScopeProvider, because it is dynamic (jotai even supports calling if/else inside atom.read!). That's how jotai is implemented, nothing could be statically analyzed until the atom is used (it also avoids unnecessary computations).

Apart from the explicitly scoped atoms, ScopeProvider knows nothing about the implicit dependency. That's the fundamental constraint, I think it may be impossible to solve (think of an if/else inside atom.read). In our mind, we know clearly which atom an atom depends on when wrapping with ScopeProvider, but JavaScript does not know it until we call it.

dmaskasky commented 4 months ago

I changed the proposal after thinking of on this more.

Did you read my current porposal? I don't think this is an issue anymore.

yf-yang commented 4 months ago

OK, then let's go back to the other setting:

yf-yang commented 4 months ago

So the rule would be: If a derived atom is scoped, then we implicitly creates a copy for all of its dependency atoms. Those copies of dependencies cannot be accessed at all.

Consequently, if both a derived atom and one of its dependency atom are scoped, then each one are independent copies of the whole dependency tree.

That rule is sound, but it introduces breaking change. Consider this case: baseAtom -> derivedAtom -> derivedDerivedAtom If base is scoped, then in current implementation, when accessing derived and derivedDerived, both will access the scoped one. However, if base and derived are both scoped, they are two copies, we don't know which copy derivedDerived should access, so the only sound rule would be β€”β€” derivedDerived should access the global one.

That is a breaking change, so we may need to implement an additional variant of ScopeProvider, if developers want to scope an atom, no matter it is primitive or derived, they should scope the one they want to use, and all the scoped atoms are independent and have their own state values.

dmaskasky commented 4 months ago

Clarifying a few points.

If a derived atom is scoped, then we implicitly create emplace a copy for all of its dependency atoms for that scope by storing the copies in the scope's WeakSet using origAtom as key.

Consequently, if both a derived atom and one of its dependency atom are scoped, then each one are independent copies of the whole dependency tree would share the copies of the dependency tree.

That rule is sound

Given these clarifications, do you still think this rule is sound?

but it introduces breaking change.

I don't think this is a breaking change.

In new implementation, both will access the scoped one. I believe this would be a natural extension of current implementation.

yf-yang commented 4 months ago

Hmmm, so that would be

If baseAtom is scoped, then both derivedAtom should access the copy (to be backward compatible) If derivedAtom1 is scoped, then derivedAtom1 should access the copy, derivedAtom2 should access the global one.

Then, let's talk about implementation, from derivedAtom2's perspective, it does not know if baseAtom is scoped or not, so it would check if the WeakMap contains baseAtom's original atom as key, right? Then it falls back to the order problem, if derivedAtom1 is scoped and

How do we guarantee derivedAtom2 accesses the right baseAtom here?

dmaskasky commented 4 months ago

There is no order dependence issue.

The weakMap is for:


❌ baseAtom is not scoped. βœ… derivedAtom1 is scoped. ❌ derivedAtom2 is not scoped.

Since derived2 is not scoped, it must use the un-scoped version of baseAtom.


βœ… baseAtom is scoped. ❌ derivedAtom1 is not scoped. ❌ derivedAtom2 is not scoped.

If baseAtom is scoped both derived1 and derived2 will use the scoped version.

This is same as before.


❌ baseAtom is not scoped. βœ… derivedAtom1 is scoped. βœ… derivedAtom2 is scoped.

If derived1 and derived2 are both scoped, both derived1 and derived2 will use the same scoped baseAtom.

yf-yang commented 4 months ago

LGTM, let me give it a try.

yf-yang commented 4 months ago

@dmaskasky You can play with this one for now.

npm i https://pkg.csb.dev/jotaijs/jotai-scope/commit/e858a7ef/jotai-scope
dmaskasky commented 4 months ago

Thank you. I'll take a look on Tuesday.

yf-yang commented 4 months ago

Another issue: Suppose

Parent Scope: derived1 is scoped Nested Scope: derived2 is scoped

Then, in nested scope

dmaskasky commented 4 months ago

I might not understand what you are asking, but here's trying.

Below is the configuration I think you are describing.

Some interesting things to note:


const baseAtom = atom('base')
const derivedAtom1 = atom((get) => {
  return `derived1 ${get(baseAtom)}`
}, (get, set, value) => {
  // sets the baseAtom scoped to derivedAtom1
  set(baseAtom, value)
})
const derivedAtom2 = atom((get) => {
  return `derived2 ${get(baseAtom)}`
}, (get, set, value) => {
  // sets the baseAtom scoped to derivedAtom2
  set(baseAtom, value)
})
const final = atom((get) => {
  const derived1 = get(derivedAtom1)
  const derived2 = get(derivedAtom2)
  return `final ${derived1} ${derived2}`
})
function ParentComponent() {
  useHydrateAtoms([[derivedAtom1, 'parent']])

  useAtom(baseAtom) // 'base'

  // 'derived1 parent
  useAtom(derivedAtom1)

  // 'derived2 base
  useAtom(derivedAtom2)

  // 'final derived1 parent derived2 base'
  useAtom(finalAtom)

  ...
}

function NestedComponent() {
  useHydrateAtoms([[derivedAtom2, 'parent']])

  useAtom(baseAtom) // 'base'

  // 'derived1 parent'
  useAtom(derivedAtom1)

  // 'derived2 nested'
  useAtom(derivedAtom2)

  // 'final derived1 parent derived2 nested'
  useAtom(finalAtom)
}

function App() {
  return (
    <ScopeProvider
      atoms={[derivedAtom1]}
    >
      <ParentComponent />
      <ScopeProvider
        atoms={[derivedAtom2]}
      >
        <NestedComponent />
      </ScopeProvider>
    </ScopeProvider>
  )
}
yf-yang commented 4 months ago

OK, yes, I mean to ask which base atom that derived1/derived2 will access in the nested scope.

"Derived1 accesses the scoped one in parent" is intuitive, but needs some refactor.

Another question, in the same setting, if derived1 is scoped in parent and base is scoped in nested, which base atom will derived1 access in nested?

dmaskasky commented 4 months ago

Another question, in the same setting, if derived1 is scoped in parent and base is scoped in nested, which base atom will derived1 access in nested?

Good question! Which do you think is more predictable?

Option 1: Since the developer has explicitly set the baseAtom to be scoped. This is not the same as the implicit scoping of the baseAtom by derived1. Explicit scoping should always take higher priority. Therefore, everything that uses baseAtom in the scope will now use the scoped base atom.

Option 2: It's interesting to think of scopes as closures. This would be familiar to developers but would still take some getting used to and would be a departure from current behavior. It would also require all atoms that share baseAtom be passed to the ScopeProvider. I'm not confident the ergonomics would be acceptable with this behavior. Furthermore, it disallows mixing scoped and unscoped atoms in the same derived atom.


The difference seems subjective at face value but has deeper implications that become obvious once you analyze the patterns and behaviors it implies.

Personally, I think we should go with Option 1. It is non-breaking and the most simplistic of the two.

I don't think Option 2 will work nor would it be worth exposing additional API to allow for opt-in.

yf-yang commented 4 months ago

Take previous rule in mind (if baseAtom is not scoped in nested, then derived1 goes to parent scope), we need to first search if derived1 is scoped in the parent, then search its dependencies.

That behavior implies a rule: Direct scope has higher priority than nested scope (Can you make a clearer description? I don't know how to phrase). So come back to this example, when using derived1, derived1's scope is first resolved, and we find one, then we use parent scope's base atom.

So, if baseAtom is used, it will access nested. If derived1 is user, it will access parent's. That's different from Option 1.

Apart from that, I am not sure if that behavior is easy to implement. Needs some investigation.

dmaskasky commented 4 months ago

we need to first search if derived1 is scoped in the parent, then search its dependencies.

I think this can be done in O(1). You just first check if the atom dependency is explicitly scoped otherwise use the atom dependency associated to the atom's scope.

Can you make a clearer description?

The closest scoped atom is the winner. ☺️

I don't know how to phrase). So come back to this example, when using derived1, derived1's scope is first resolved, and we find one, then we use parent scope's base atom.

So, if baseAtom is used, it will access nested. If derived1 is used, it will access parent's. That's different from Option 1.

I don't see how this is different from Option 1. If derived internally uses base (call it internal base), and if derived is explicitly scoped and base is not, then internal base is implicitly scoped.

Then if in nested base is scoped, the closest store wins so nested base is closer than internal base. The internal base atom is shadowed by nested base.

This wouldn't be computationaly expensive to determine this.

dmaskasky commented 4 months ago

Thank you. πŸ™Œ