-
Notifications
You must be signed in to change notification settings - Fork 75
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
Suggestions for custom Datatypes #112
Comments
As fun as it is to read my writing, code sometimes speaks better than words, so I've got a commit that kind of spells out what I mean, as well as a test that covers the issue I described. |
Hey @mmacaula Is this still relevant? Is that bug you found in |
As far as i know. |
Yes, this is still a bug. I came across a related bug today - when using This causes problems when using a default value on a view property, and then using bindings on that property: because At first I thought, "oh well just replace The real problem is that there are side-effects in the I don't know if I have an opinion on the other suggestions here, but as a start I'm +1 on moving side-effects out of |
If there's interest in getting this fixed, I think #145 is the best place to start, it just never got merged. I think we need to update it but I believe that should fix it. As with the other suggestions, I agree, I think we can hold off on them. :) |
Hi all,
Been using & (converting a backbone app) and really like it. When I was working on #103 I was staring at the code a lot and came up with some suggestions to make datatypes more powerful and consistent. Most of this comes from a potential bug I noticed in State, which kind of caused a trickle of ideas on making datatypes even more powerful. I'll lay out the bug first, then my ideas. Feel free to rip this apart, just wanted to get a discussion going. If it's a good idea, great, if not, just close this issue.
First off, technically there's a bug in in
set
link to line when using a value inprops
of typestate
. In theisEqual
function, it calls the comparator, but the comparator for thestate
datatype has extra code in it besides just comparing. It does some work tearing down the old listeners and setting up new ones. 99% of cases this is fine, but if thesetOnce
property is set and an error gets thrown just a few lines later, then you've got your events all messed up, pointing to the new (invalid) state when the old one is still what's on the parent state. I'm happy to create an issue just for this bug, but I'll describe the cascade of thoughts that followed.So, at the same time I was doing a lot of work with
children
andcollections
and got mixed up and confused about them. They're not really designed to be swappable, but I found myself wanting them to be so. We have a lot of unit tests in our app that build our models and compose them usingset
. I thought, why don't I just create a swappable datatype, likestate
but typed to my model and it sets up that parent relationship for me? Something like this:The problem though is that the
set
method of the datatype isn't bound to the parent state. So that's my first suggestion, allowset
in the datatype to be bound to the parent state. This would look a lot like the _getCompareForType but forset
. This would allow my fictional datatype above to become possible.The next suggestion would be to add another method to datatypes, called
onSet
or something that would be called when all validations and type checking and comparison checking is done and the value is going to be placed on the state itself. This method would be used to handle setting up events or calling initialization methods. The code currently in thestate
dataTypescompare
method would be moved here. Ultimately this could become confusing.set
andonSet
seem too close, and I would suggest that we renameset
to something likeprepare
orparseDataType
or something, I'm horrible with naming. That would of course be a breaking change to the API...Then, just thinking outside the box here, with these methods, something really cool begins to be possible. The
children
andcollections
of the current state could become just a default datatype in props. You wouldn't need to specify different hashes for them, and all the special code inamersand-state
for collections and children (initialization, sections in set, etc) would be delegated to these default types. If you'd like swappable children, there can be a datatype for that. Those datatypes can be pulled out to separate ampersand tiny modules that people can just mixin when they want. Obviously this last idea is a really big change and I'm probably missing something that would not make it possible, but just wanted to throw the ideas out there.The text was updated successfully, but these errors were encountered: