-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
customAtoms are not properly scoped #28
Comments
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. |
If our design choice is to prohibit scoping derived atoms, we might be able to warn it by checking the read function. Wait, it's much easier: 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
} |
Well, I actually don't think it's a very good idea. We should hide base atoms to avoid leaking anything. |
Wouldn't the store have access to the atom's dependencies? Could the store provide some api to allow derived atoms to scope? |
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. |
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. |
Yeah, that's the tough one. |
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.
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?
For custom functions, I think people could decide themselves to return the base atom, if they really need it. |
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. |
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. |
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. |
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.
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're free to write the conventions and api. Surely a solution to this problem is not impossible. |
|
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. We need to figure out if those two problems can be solved gracefully before proceeding to |
|
This order dependence seems like an implementation detail. If derivedAtom1 is scoped, then baseAtom is scoped. This is regardless of whether 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 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. |
Scoping Writable Atomsconst 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 Atomsconst 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>
)
} |
I know what you want, but unfortunately, the dependency computation happens when you call 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. |
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. |
OK, then let's go back to the other setting:
|
So the rule would be: 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: 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. |
Clarifying a few points. If a derived atom is scoped, then we implicitly Consequently, if both a derived atom and one of its dependency atom are scoped, then each
Given these clarifications, do you still think this rule is sound?
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. |
Hmmm, so that would be
If baseAtom is scoped, then both derivedAtom should access the copy (to be backward compatible) 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?
How do we guarantee derivedAtom2 accesses the right baseAtom here? |
There is no order dependence issue. The weakMap is for:
❌ baseAtom is not scoped. Since derived2 is not scoped, it must use the un-scoped version of baseAtom. ✅ baseAtom is scoped. If baseAtom is scoped both derived1 and derived2 will use the scoped version. This is same as before. ❌ baseAtom is not scoped. If derived1 and derived2 are both scoped, both derived1 and derived2 will use the same scoped baseAtom. |
LGTM, let me give it a try. |
@dmaskasky You can play with this one for now. npm i https://pkg.csb.dev/jotaijs/jotai-scope/commit/e858a7ef/jotai-scope |
Thank you. I'll take a look on Tuesday. |
Another issue:
Parent Scope: derived1 is scoped Then, in nested scope
|
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>
)
} |
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? |
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. |
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. |
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.
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.
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. |
Thank you. 🙌 |
Summary
Custom atoms are not scoped because their base atom is not scoped. Is there a workaround for this use-case?
CodeSandbox Example
https://codesandbox.io/p/sandbox/inspiring-grothendieck-gcc2jt
Other issues
Not sure if this is related to #24. I don't think it is.
Proposal 💡
#28 (comment)
Scope should apply to dependency trees.
This change will make understanding what is scoped a lot easier.
Scoping Writable Atoms
Revisiting Scoping Base Atoms
The text was updated successfully, but these errors were encountered: