-
-
Notifications
You must be signed in to change notification settings - Fork 562
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
[RFC] useAtomValue and useSetAtom APIs in core #885
Comments
Happy to work on this @dai-shi |
@Thisen No rush. It will still take time. The src part is trivial. The docs part is bigger. |
@dai-shi I'd prefer It's because it translates to plain English. "Use the atoms value" Versus "Use an update atom"
To help illustrates my point the API shouldn't be |
Hm, that Any other comments from someone else? |
I think |
It seems like there will be none that everyone would be happy with. @Thisen If you would really like to work on it, you can start working on a draft pr. Note that there's no ETA, and the pr will be long lived, like for months. |
I favor it for its brevity as well. 🙂 |
Found an RFC in recoil: facebookexperimental/Recoil#804 Community-wise, it's not a bad idea to follow with It's unfortunate, we had internally My current options are: Anyone has/change a vote? |
|
@dai-shi I retract my original opposition. I changed my mind because the React hooks community is firmly behind naming these functions as
I think the Jotai API documentation should also make sure to continue that convention, and keep naming consistent across both sides of the assignment.
👍 for |
Looks like reaching to an agreement. Just a note why I didn't choose const countAtom = atom(0, (get, set, action) => {
if (action === 'INC') {
set(countAtom, get(countAtom) + 1))
} else {
set(countAtom, 0)
}
})
// in component
const [count, dispatch] = useAtom(countAtom)
const dispatch = useDispatch(countAtom) // weird for `useState` case
const dispatch = useUpdateAtom(countAtom) // so, chose this
const dispatch = useSetAtom(countAtom) // probably, this looks okay |
@Thisen may not be available soon, I'll create a PR only for code meanwhile. (we want to include it in the next minor.) |
When we designed the API, we wanted to make it small and the core provides only three functions:
atom
,useAtom
, andProvider
.Later,
Provider
becomes optional.Now, we have
useAtomValue
anduseUpdateAtom
exported fromjotai/utils
.Implementation-wise, having them in core makes more sense.
We want to keep claiming that jotai has small API surfaces, but adding those two in core should be fine.
There will be two basic APIs and three additional APIs.
atom
,useAtom
Provider
,useAtomValue
,useSetAtom
notes:
useUpdateAtom
touseSetAtom
is intentional. Should have done this from the beginning.useAtomValue
anduseUpdateAtom
fromjotai/utils
for backward compatibility.The text was updated successfully, but these errors were encountered: