connectToStores with an Array of stores

[martintietz]

Right now there is an issue if you are using the FluxMixin (the same should be true for FluxComponent) with an array and a single stateGetter like this:

FluxMixin(['storeA', 'storeB', 'storeC'], function(store) {
...
})

In this case the callback will be called 3 times (each time with one with the stores as an argument). It would be better if it could be called only one time with all 3 stores being arguments of the callback. So that we could use the mixin like this:

FluxMixin(['storeA', 'storeB', 'storeC'], function(storeA, storeB, storeC) {
...
})

This would need some refactoring inside the connectToStores and getStoreState functions.

[acdlite]

@martintietz That's a good idea. Basically a reduce operation. It would require a bit of refactoring, though, since getStoreState() assumes a one-to-one relationship between stores and state getters.

[tappleby]

Will this still need to support the 1 call per store to the stateGetter function?

If so one option could be to check the number of arguments on the stateGetter using Function.length (similar to how react-router does with willTransitionTo)

[acdlite]

@tappleby Not sure I understand your question. The single key and object forms of connectToStores() will continue to work the exact same way. This would only affect the array form.

Only change I'd make to @martintietz's suggested API above is to pass an array of stores to the getter function, rather than spread them across as arguments.

[tappleby]

Sorry, my question was when using the array form would there be some cases where the user wants the stateGetter to be called multiple times (once for each store)?

Initially I was thinking multiple calls would be required when using the defaultStateGetter FluxMixin(['storeA', 'storeB', 'storeC']), but that can be handled with a state getter that performs the reduce you mentioned.

My only issue with an array of stores instead of the spread is its not as clean when accessing the stores inside the getter.

[acdlite]

If you're not dealing with them individually, you'd rather them be an array so you can do stores.reduce(). It's mostly an aesthetic thing, though, because with ES6, you can always do let stores = Array.from(arguments), or conversely, let [ storeA, storeB, storeC ] = stores.

[tappleby]

Ah thats not so bad then. I was picturing the reduce using something like (...stores) => stores.reduce(...).

[acdlite]

Yeah, even better. I still kinda like the symmetry of the taking in an array of store keys and passing an array of stores, but either way makes sense to me.

[tappleby]

Yeah I do like the symmetry aspect.

from a functionality point of view, at least for my use cases, accessing the stores individually will be a lot more common then performing a reduce so it could get repetitive having to do:

function (stores) {
  let [ storeA, storeB, storeC ] = stores;
  ...
}

Also for those not using ES6 it gets a bit more verbose.

[acdlite]

You could actually also do this:

function ([storeA, storeB, storeC]) {
  ...
}

I could really go either way on this. Can you think of a similar api from another library perhaps? I know there must be one, I just can't think of any off the top of my head.

[tappleby]

That works. I used to have issues with that syntax and jstransform but appears to be resolved now (should probably just switch to babel anyways).

I cant think of a similar api offhand, will see if anything comes up in the morning.

With the ES6 syntax, I'm impartial to either. For ES5 its a bit cleaner as separate parameters or arguments for reduce. not sure about TypeScript of CoffeeScript.

[gaearon]

What about object destructuring?

FluxMixin({ storeA, storeB, storeC }, function({ storeA, storeB, storeC }, props) {
...
})

(Pardon if I misunderstand how this mixin works and you're discussing something else entirely!)

[acdlite]

@gaearon Destructuring is what I was suggesting above, except for arrays, not objects.

There are three ways you can call FluxMixin / connectToStores(). In the object form, the keys map to individual store getters, so you don't need the second parameter. We're discussing changing the array form so that you can specify a single state getter that is passed all the stores, as either an array or spread across the args.

It's this:

FluxMixin(['storeA', 'storeB', 'storeC'], function(storeA, storeB, storeC) { ... }

versus

FluxMixin(['storeA', 'storeB', 'storeC'], function(stores) { ... }

ES6 makes the difference mostly cosmetic — in the first form you could use spread to transform the args into an array, and in the second form you could use array destructuring to transform the array into "args".

[acdlite]

@gaearon Oh now I think I see what your actual point was. Now that we've discussed passing props to state getters over in #31, the array form really is necessary so we can pass extra parameters at the end:

FluxMixin([ 'storeA', 'storeB', 'storeC' ], function([ storeA, storeB, storeC ], props) { ... })

[gaearon]

Oh. I just realized why you're using strings there :-).

[acdlite]

Added in #73 by @tappleby. Will be released as part of 3.0.