fix(store): remove dev3 methods and add dev4 methods only in store2 #2484
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: -3.01 kB (-3%) Total Size: 90.2 kB
ℹ️ View Unchanged
|
|
| Playground | Link |
|---|---|
| React demo | https://livecodes.io?x=id/6VG5Q9Y4P |
See documentations for usage instructions.
dai-shi
changed the title
| dev4_get_internal_weak_map: () => atomStateMap, | ||
| dev4_override_method: (key, fn) => { | ||
| ;(store as any)[key] = fn |
There was a problem hiding this comment.
Nice! A few questions
-
With this approach users would probably need to
composeWithDevTools(store || getDefaultStore())and supply it via<Provider/>to their application. So the DevTools would no longer support the provider-less mode unless if we keepcomposeWithDevToolsfunction within the main lib. Any thoughts around this? 🤔 -
What would be the best way to wait for atoms resolving a promise (in DevTools)? i.e. values changing from sync -> async or async -> async (new promise). This did not exist in v1 btw, but would be nice to
include it in v2 -
Could we still keep
mountedAtomsaround? Perhaps we could introducedev4_get_internal_mounted_atom_map? If you feel strongly about not including it, then I could start with keeping references to the atoms changed withsubscribe/unsubscribe🤔 -
Do we still need
dev4prefix or should we call itdev_get_internal_weak_mapetc.? From Jotai DevTools perspective, exposingimport { DevTools } from 'jotai-devtools/experimentalmight make more sense that eliminates any conflict with thedevvsdev4
There was a problem hiding this comment.
- With this approach users would probably need to
composeWithDevTools(store || getDefaultStore())and supply it via<Provider/>to their application. So the DevTools would no longer support the provider-less mode unless if we keepcomposeWithDevToolsfunction within the main lib. Any thoughts around this?
I thought without dev4_override_method, that's the case, and hoped dev4_override_method could work it around.
There was a problem hiding this comment.
2. What would be the best way to wait for atoms resolving a promise (in DevTools)?
I assume you can simply await the promise value from store.get().
There was a problem hiding this comment.
3. Could we still keep
mountedAtomsaround?
Hmm, that was one of the biggest things, I would like to avoid in dev4. Otherwise, we can re-consider dev3.
There was a problem hiding this comment.
4. Do we still need
dev4prefix or
I don't understand what conflicts. I prefer the revision number for clearer communication. (we may have dev5 in the future.)
There was a problem hiding this comment.
I thought without dev4_override_method, that's the case, and hoped dev4_override_method could work it around.
Since it is updating the reference of the store it should work with the default store too 🤔
Hmm, that was one of the biggest things, I would like to avoid in dev4. Otherwise, we can re-consider dev3.
I can work around it by patching store.set and manually keeping track of the atoms by sub and unsub. Not sure if I'm missing any edge cases here.
I don't understand what conflicts.
Mostly TypeScript related, I'm trying to avoid the number of type guards 😅 but I can circumvent it.
| type Store = | ||
| | (PrdStore & Partial<DevStoreRev2>) | ||
| | (PrdStore & DevStoreRev2 & DevStoreRev3) | ||
| type Store = PrdStore | (PrdStore & DevStoreRev4) |
There was a problem hiding this comment.
Any chance we could export PrdStore & DevStoreRev4?
Thanks, and we can modify/improve it (and adding more tests for |
base pr #2474