Closed stephenh closed 1 year ago
The latest updates on your projects. Learn more about Vercel for Git ↗︎
Name | Status | Preview | Comments | Updated |
---|---|---|---|---|
valtio | ✅ Ready (Inspect) | Visit Preview | 💬 Add your feedback | Jan 31, 2023 at 0:12AM (UTC) |
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.
Latest deployment of this branch, based on commit 3aa09fa46cc27632d6d7b19e6d6a0bc3bf3c75b7:
Sandbox | Source |
---|---|
React | Configuration |
React Typescript | Configuration |
React Browserify | Configuration |
React Snowpack | Configuration |
React Parcel | Configuration |
But not sub, because it reminds me of subscribe function which returns unsubscribe function.
Ah yeah, that's fair; wdyt about add
?
Because it's just adding the proxy to the list of proxies to watch...
...totally non-serious musing, but fwiw I'm wondering if a better API for watch would have been:
watch(proxy, () => { ...logic... });
watch([proxy1, proxy2], () => { ... });
Because I think it makes it more clear that we're passing proxy
solely for the purpose of getting it subscribed, and we don't need to be returned/passed back a special/decorated version of the proxy
.
...granted, maybe someday get
ends up returning a snapshot. :-) :shrug:
Yeah, let's leave it as get
for now. I understand the original confusion, but it's fairly fine. sub
or add
seems more confusing.
watch(proxy, () => { ...logic... });
Is it just same as subscribe
?
watch([proxy1, proxy2], () => { ... });
It's actually much easier to implement. watch
is more capable than that. It allows to use get
conditionally, and will auto subscribe/unsubscribe on demand.
(tbh, if watch
isn't widely used, we could simply drop it. it feels suboptimal. we could add a new util with usage tracking. might be as a third-party package. see also #609 discussion.)
sub or add seems more confusing.
Okay, changed back to get
.
Do the rest of the doc updates look okay?
It allows to use get conditionally, and will auto subscribe/unsubscribe on demand.
Right, I know my proposal was less flexible, i.e. it could not conditionally watch only proxy1
or proxy2
, but I liked the clarity of "param1 --> the proxies I'm watching, refire whenever they mutate"; "param2 --> the code that runs when that happens".
As-is, with the get
interspersed in the code block, it made me jump to the conclusion that watch
had mobx/useSnapshot
-style usage tracking where the properties accessed by the get
return values really mattered.
Moving to a two-arg style, which you're right is exactly like subscribe
, imo just makes the mental model clearer as to what is happening.
All that said, of course/yeah totally fine to keep as-is. Maybe someday get
starts returning snapshots and then the current API will make a ton of sense. :-)
Fwiw I didn't really follow the proposed reaction
/ track
API, but :shrug: that's fine...
Per your comment:
That's why adding tracking in vanilla.ts is unacceptable.
I'll probably use valtio from the convenience of useSnapshot
anyway, so I don't think it really matters to me, but fwiw personally I think usage based tracking is a really core component to a mature observer library, so would nudge for snapshots to eventually make their way into vanilla, but, again, probably doesn't really matter/totally up to you.
Thanks for the discussion!
it made me jump to the conclusion that
watch
had mobx/useSnapshot
-style usage tracking
I see. I feel like I'm failing to explain the basic conception. Valtio is built with very different mental model. It's not like observer/observable practice. What valtio core (valtio/vanilla) does is mutable state -> immutable state conversion. It's close to the mental model of Immutable.js or Immer.
Only, useSnapshot (in valtio/react) does tracking. And, it confuses, I know. So, while typical observer pattern does the whole task altogether, valtio split the task into two parts. It was born because of the immutability requirement of React, but I notice valtio/vanilla itself is kind of new approach.
Anyway, it's the design decision, valtio core doesn't do tracking, does only one thing. Adding tracking-capable watch in valtio/tracking
or somewhere else might make sense in that sense. (but then, it confuses what is "vanilla"..., hmm maybe it's fine there.)
It's probably doc issue, like it should clearly say, something is different from say mobx. We could name the hook as useSnapshotWithTracking
, but it's too long. At the same time, we want people to use it without knowing too much in detail.
Thanks for your feedback!
Maybe someday
get
starts returning snapshots and then the current API will make a ton of sense.
On second thought, for one reason, I like the idea, because current watch and subscribe is too close in their capability. Just conditional subscription. Adding tracking will expand its usage.
The remaining concern is it's very hard to implement. We did once in proxyWithComputed actually, but the code is hardly readable (now we recommend combining proxy-memoize
). So, probably, while subscribeWithTracking
is fairly easy to implement (it's like useSnapshot), watchWithTracking
will be very very complicated. Not sure if it's worth, given that we don't know how many people would use it.
So, in conclusion, I'd suggest someone to try it as a third party library for a start.
I feel like I'm failing to explain the basic conception.
Ah yeah, sorry, that's my fault; I do get that valtio is fundamentally different, I was just attempting to articulate how the current watch/get API, to me, had encouraged me as a still-newbie to assume "it's like mobx", even though yep I get that it's not.
It's probably doc issue, like it should clearly say, something is different from say mobx
My hope is to contribute some docs in that regard, once I deeply understand valtio, but I'm only about ~1/3rd of the way there. :-)
because current watch and subscribe is too close in their capability. Just conditional subscription. Adding tracking will expand its usage.
Ah nice! Right! I think that's a great way of rationalizing the additional complexity (but increased power/utility to users).
Thanks for accepting the PR!
Summary
Just clarifying the non-usage tracking nature of
watch
.Check List
yarn run prettier
for formatting code and docs