fluxxor icon indicating copy to clipboard operation
fluxxor copied to clipboard

Allow defining store mixins via a spec syntax similar to Component mixins.

Open STRML opened this issue 10 years ago • 10 comments

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 createStore so they can be overridden by users if necessary; for instance, I could see users wanting to define a different merge policy, such as deep merge.

STRML avatar Jun 28 '14 21:06 STRML

If you are good with this syntax, I would be happy to add to site/contents/documentation/stores.md.

STRML avatar Jun 28 '14 21:06 STRML

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.

BinaryMuse avatar Jun 29 '14 01:06 BinaryMuse

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 fetch methods and can even automatically connect to realtime data via a mux-demux stream. This would be much more difficult to do without mixins.

STRML avatar Sep 12 '14 22:09 STRML

This is awesome, mixins are a must for stores! +1!

evindor avatar Feb 05 '15 18:02 evindor

@STRML I just merged this against current master in my fork and I had to use component.bindActions instead of merge here: https://github.com/BinaryMuse/fluxxor/pull/37/files#diff-32584de7a758f3cebf1bd66000a8b7caR47

I think .bindActions should do the job of merging or am I missing something? @STRML you've put .merge here for a reason, right?

evindor avatar Feb 06 '15 08:02 evindor

@evindor .bindActions may indeed work, in this case I thought it made more sense to simply merge the object and let Fluxxor call bindActions when initializing the store. I am not sure why it is not working for you; which version of Fluxxor did you merge this to?

STRML avatar Feb 06 '15 08:02 STRML

@STRML merged to current master.

evindor avatar Feb 06 '15 08:02 evindor

@STRML Seems like Fluxxor just never calls .bindActions anywhere else. Anyway it works for me now. Hope we'll get it merged soon!

evindor avatar Feb 06 '15 08:02 evindor

@evindor I've rebased to latest master and it passes tests again.

When I wrote this PR, around version 1.4, createStore didn't call .bindActions but instead just merged into the __actions__ object. Around 1.5 it changed the behavior. This PR now uses the new behavior so it should work just fine.

STRML avatar Feb 06 '15 09:02 STRML

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 React.createClass), with the goal of utilizing more idiomatic JS patterns to implement that functionality in the future (or using composition instead).

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.

BinaryMuse avatar Apr 05 '15 21:04 BinaryMuse