Allow defining store mixins via a spec syntax similar to Component mixins. #37
Conversation
|
If you are good with this syntax, I would be happy to add to |
|
Hey, @STRML, thanks! This is cool stuff. I'm going to tag this for the 2.0 release for now, as there are a couple pending decisions around the 2.0 API I'm working through and I want this to be part of that thought process. |
STRML
force-pushed
the
storeMixins
branch
2 times, most recently
from
5f35b2b
to
b76ac74
Compare
|
Went ahead and rebased this to master. Just wanted to let you know I've been using it for about 4mo, 2 of those in production, and it's working pretty well. I've been using a combination of mixins like this: mixins: [
AutoRefreshMixin({
subscriptionName: 'trade',
storeKey: 'trades'
}),
RestBindingMixin({
storeKey: 'trades',
restEndpoint: '/trade/getRecent',
restParams: {
count: 100
}
})
]It's been hugely valuable in my workflow - my stores have auto-generated |
|
This is awesome, mixins are a must for stores! +1! |
|
@STRML I just merged this against current master in my fork and I had to use I think |
|
@evindor |
|
@STRML merged to current master. |
|
@STRML Seems like Fluxxor just never calls |
|
@evindor I've rebased to latest master and it passes tests again. When I wrote this PR, around version |
|
Just wanted to pop in for a quick update here. There's no doubt that this functionality is quite useful from a reusable abstractions standpoint, but I've always been hesitant to merge it in due to the complexity that mixins bring (similar to the complexity they brought to React). As you probably know, there's been a big push toward native ES6 classes from the React team, and using native classes in React 0.13 precludes the use of mixins (as provided by Furthermore, a future version of Fluxxor is likely to provide a more flexible method of store creation, which will include less "magic," and I'm not sure how this particular feature will fit in to that effort. So, for now, I'm going to leave this unmerged, but rest assured that making reusable store patterns easier to implement is definitely on my mind going forward. |
| if (!_isObject(spec[key])) { | ||
| throw new Error("Actions must be defined as an object."); | ||
| } | ||
| component.bindActions(spec[key]); |
There was a problem hiding this comment.
Just realized, we need to chain these, if both a Store and its mixins are listening to the same action, only the Store will fire the listener.
Fixes #32.
I found that I was defining similar functionality on a number of stores, especially if they listened to the same data.
Similar to React components, this will merge actions, chain functions, and attempt to merge objects/arrays. It will not, however, merge the result of functions (like createMergedResultFunction) as we have no such use case.
All the mixin functions are assigned directly to
createStoreso they can be overridden by users if necessary; for instance, I could see users wanting to define a differentmergepolicy, such as deep merge.