-
Notifications
You must be signed in to change notification settings - Fork 88
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
Why don't store models leverage property descriptors too? #209
Comments
I think I don't fully understand your case. Can you provide an example code, which show the problem? |
For instance it would be more powerful if instead of: const Set: Model<Set> = {
id: true,
weight: 0,
distance: 0,
reps: 0,
time: 0,
angle: store.value(
0,
(v) => v >= 0 && v <= 90,
'Only angles of 0-90 degrees are supported',
),
} I could write: const Set: Model<Set> = {
id: true,
weight: 0,
distance: 0,
reps: 0,
time: 0,
angle: {
get: ...
set(host, value) => {
if (ok(value)) {
return value
}
if (recoverable(value) {
return fix(value)
}
throw new Error('oops')
}
},
} |
Thanks for taking so much time btw - I'm still trying to learn hybrids to a level where I can actually implement an application in it so I might be missing some rather obvious stuff. That being said I really like the model (and 0-dep approach) thus far and I figure it's best to surface these things to make it easier for people when hybrids makes it big ;) |
The store models rely on the value type, and by the design, they must be serializable to JSON-like structure. The store would not be able to detect (or even check) if the property defined with get/set methods fits the requirements. However, you are free to make that validation in your const Model = {
value: 0,
[store.connect]: {
get() { ... },
set(_, model, keys) {
if (keys.includes('value') && !validate(model.value)) {
const err = new Error("Validation error");
err.errors = {
value: "Value must be ...",
};
throw err;
}
...
},
},
}; You can find more info about throwing errors here: https://hybrids.js.org/#/store/usage?id=storeerror Additionally, you are free to create a computed property, which returns whatever you want, as this is not serialized (I know, that this is not what you mean here, but I say it to highlight what's possible). |
Makes sense from a design perspective, though it is a tad cumbersome from a dev perspective since there are now 2 spots where you handle any given value in a store (its own property and the store.connect property). Perhaps a better place to handle this is in an exported setter "action" function similar to what Pinia has? Am I correct in assuming those won't need special framework support and essentially amount to a design pattern consisting of an exported function from whichever module you implement your store/model in? If so, is there any way to ensure that the field is only set through that function (private fields are not valid inside POJOs)? Is this a common restriction (being JSON serializable)? I suspect so, I've just never delved into state management library internals. I'm only asking because I don't recall bumping into this problem. PS: Do you guys have a place where you have more casual discussions that don't need issue tracking? E.g. I would like to slowly start contributing typescript "translations" for all the docs in and would like to sync up on the best approach to doing that. Or asking silly questions about routing patterns etc. |
I have never thought before about adding to store models setters of the properties. It is something that I must rethink how it would be possible to integrate with the current API. One thing that comes to my mind is to extend The
This is also an architecture decision to make it easier to send over the wire, send to different types of data sources. The |
Personally I think people are far more used to the concept of "action" functions - see all the "increment" examples in different state management libraries. The kicker is figuring out how to make the properties only editable through those (probably through some sort of opt-in mechanism). + it's far more flexible. That said, looking forward to whatever you come up with should you choose to work on this :) |
I'm sure there's a good technical reason - I just find myself reaching for the niceties of custom elements with store models, but they're just not there. E.g. instead of the power of setter functions we have
store.validate
that can only reject invalid inputs.The text was updated successfully, but these errors were encountered: