feat: dev store with unstable_derive #2852
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
|
Size Change: +1.14 kB (+1.26%) Total Size: 92.1 kB
ℹ️ View Unchanged
|
|
| Playground | Link |
|---|---|
| React demo | https://livecodes.io?x=id/2K6HWC4WD |
See documentations for usage instructions.
src/vanilla/store.ts
Outdated
| }), | ||
| dev4_get_mounted_atoms: () => debugMountedAtoms, | ||
| dev4_restore_atoms: (values) => { | ||
| throw new Error('TODO: not implemented yet' || values) |
There was a problem hiding this comment.
This probably requires "batch write".
There was a problem hiding this comment.
#2827 introduces batch inside the write function.
So after the PR lands, it could look something like:
{
...
dev4_restore_atoms: (values) => {
store.set(atom(null, (get, set) => {
values.forEach(([a, v]) => set(a, v))
})
},
}
|
/ecosystem-ci run |
Ecosystem CI Output |
|
@arjunvegda Let's add jotai-devtools to https://github.com/jotaijs/jotai-ecosystem-ci ! Can you open a PR there? |
|
Separately, I think createPending should be part of deriveStore. |
If necessary, I generally want to make functions as pure as reasonably possible. (edit: I misunderstood the question.) |
|
/ecosystem-ci run |
Ecosystem CI Output |
Yes, it's expected. Cool. |
commit: |
dai-shi
changed the title
|
/ecosystem-ci run |
Ecosystem CI Output |
src/vanilla/store.ts
Outdated
| const restoreAtom: WritableAtom<null, [], void> & | ||
| Record<typeof RESTORE_ATOM, typeof values> = { | ||
| [RESTORE_ATOM]: values, | ||
| read: () => null, | ||
| write: () => {}, | ||
| } | ||
| derivedStore.set(restoreAtom) |
There was a problem hiding this comment.
I'm curious why not simply?
store.set(atom(null, (get, set) => {
values.forEach((args) => set(...args))
})There was a problem hiding this comment.
atom in values can have a custom write function, which we need to bypass.
Hmm, it's unexpected. |
|
@arjunvegda Can you help here? My expectation is this is compatible behavior-wise, but there might be some cases to break. |
It appears that whenever a user attempts to restore a specific snapshot, the store ends up creating another snapshot in the "Time Travel" tab in DevTools UI. The intended behavior is to restore the chosen snapshot without adding any new entries to the list. This is likely because we're calling |
|
Hm, I see. Thanks!! |
|
/ecosystem-ci run |
Ecosystem CI Output |
|
🎉 Thanks @arjunvegda! I'm planning to release this next week. |
src/vanilla/store.ts
Outdated
| if (hasInitialValue(atom)) { | ||
| setter(atom as never, value) | ||
| } |
There was a problem hiding this comment.
I think this is still an issue. atom(0, () => {}), needs the init value, but its write function should not be called.
I think you will need to add createPending to unstable_derive. Then you can update the atomState and adjust batch.
There was a problem hiding this comment.
I think this is still an issue.
Thanks for the catch. I didn't notice there was a test case missing. Added in #2873.
|
/ecosystem-ci run |
Ecosystem CI Output |
| let savedGetAtomState: StoreArgs[0] | ||
| let inRestoreAtom = false | ||
| const derivedStore = store.unstable_derive( | ||
| (getAtomState, atomRead, atomWrite, atomOnMount) => { |
There was a problem hiding this comment.
For future compat.
| (getAtomState, atomRead, atomWrite, atomOnMount) => { | |
| (getAtomState, atomRead, atomWrite, ...rest) => { |
Thanks to
unstable_derive, we can implement dev store with it.