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 | Feb 5, 2023 at 5:43PM (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 aa38b3e9fce5861b7ebd9bd9810f70bbd1832b6b:
Sandbox | Source |
---|---|
React | Configuration |
React Typescript | Configuration |
React Browserify | Configuration |
React Snowpack | Configuration |
React Parcel | Configuration |
@dai-shi okay, I've rebased this, I believe addressed all of your feedback, and also made some additions to the new Computed Properties guide. Let me know if I missed any comments, or any other changes you'd like. Thanks!
it would for getters and setters, but not methods in general.
Good point; I've just taken the "always" text out for now. I agree addressing methods would be useful, but probably as a follow up PR.
I kinda wonder if maybe snapshots should just have all methods dropped from the type; that would be possible with TypeScript mapped types, dropping all methods/functions, so only properties & getters would be invocable on the snapshot.
I'm also kind of wondering about some sort of function.bind
/ apply magic that would try to invoke object/class methods with this
always bound to the proxy
. So even snap.clickMe()
would have this
be the proxy. Not sure if that is possible or desirable, but just an idea.
May I merge this, or should wait?
Yep, feel free to merge. Thanks!
I kinda wonder if maybe snapshots should just have all methods dropped from the type; that would be possible with TypeScript mapped types, dropping all methods/functions, so only properties & getters would be invocable on the snapshot.
What do you mean? This is still useful:
const state = proxy({
firstName: '...',
lastName: '...',
getFullName() {
return this.firstName + ' ' + this.lastName
}
})
Just dropping types doesn't help for JS users anyway. I mean we want the same capability for JS and TS.
I'm also kind of wondering about some sort of
function.bind
/ apply magic that would try to invoke object/class methods withthis
always bound to theproxy
. So evensnap.clickMe()
would havethis
be the proxy. Not sure if that is possible or desirable, but just an idea.
Yes, for withClass
util, I thing .bind()
could be a nice magic. But, we need to know, if a method is for mutation, or just getting values. (If we disallow methods for getting values, and only recommends getters, it might be possible. Unfortunately, it's too counter-intuitive.)
This is still useful:
Ah yeah, a non-getter method; that's true, the wrinkle with methods is that we have to trust the user to not make any side-effecting calls, i.e. they shouldn't write:
const state = proxy({
firstName: '...',
lastName: '...',
handleNewName(input) {
this.firstName = ...
}
})
const snap = snapshot(state);
const onClick = () => snap.handleNewName(...)
And so, dunno, it seems a little safer to just say "snapshots never have might-be-side-effecting methods", and nudge users to rewrite getFullName
as a getter b/c it will better communicate "you really can't modify the state here".
Ah yep, exactly as you said, "we disallow methods for getting values".
I dunno, personally I find "methods can have side effects so snapshots don't have them" to be very intuitive, but agreed that would not be immediately obvious to beginners.
I thing .bind() could be a nice magic
Nice, yeah I'll play around with it a bit when I get a chance.
Continuing the last topic a little bit.
While I kept saying this
is advanced, I don't mean to drop it. So, I'd expect this to work:
const state = proxy({
name: 'valtio',
setName(input) {
this.name = input
},
getName() {
return this.name
}
})
state.setName('react') // expected to work as `state.name = 'react'`
snapshot(state).setName('react') // runtime error as `snap.name = 'react'`
// ^ that can be `never` in TypeScript, but there's no way to guess if a method contains mutation
state.getName() // just works
snap(state).getName() // should work too as `snap.name`
Furthermore, this is valid, right?
const state = proxy({
name: 'valtio',
counter: 0,
get theName() {
++this.counter
return this.name
}
})
While I kept saying this is advanced, I don't mean to drop it
Great!
Yep, everything you outlined makes sense to me, in terms of both making sense intuitively + being how Valtio behaves today.
Just adding things that personally I have learned/think is useful to know so far.
Check List
yarn run prettier
for formatting code and docs