`compose` is not a normal compose

[jlongster]

compose is a function that normally doesn't eagerly evaluate it's arguments, but returns a new function that, when called, does the right-to-left evaluation.

It's easy if the composition returns a function that takes a single argument (I think this is the proper mathematical composition):

function compose(...funcs) {
  return x => {
    return funcs.reduceRight((composed, f) => f(composed), x);
  };
}

But usually we want to seed the application with multiple args, but it's just a little more code:

function compose(...funcs) {
  return (...args) => {
    const initialValue = funcs[funcs.length - 1].apply(null, args);
    funcs = funcs.slice(0, -1);
    return funcs.reduceRight((composed, f) => f(composed),
                            initialValue);
  };
}

This changes the pattern of wrapping createStore though. One option would be to rename this to eagerCompose or something. Not sure if you all are interested in fixing this, but it is somewhat confusing that acts different than most (all?) other compose functions.

[gaearon]

I agree, let's fix it somehow. It confused the hell out of me too on one occasion. :-P

[jlongster]

Haha. If you're not open to changing how higher-order stores work (they return a function that takes a "createStore" function), seems like the only option is the change the name. That's the main problem right now: they impose a contract on the composed functions when normally the functions don't even know they are being composed.

Even changing the name will cause breakage though, so I dunno.

[gaearon]

Haha. If you're not open to changing how higher-order stores work (they return a function that takes a "createStore" function)

I'm open to it as long as we can make dev tools and hot reloading work with some other contract.

[gaearon]

See also https://github.com/rackt/redux/issues/376 #350 If you can figure out a way to make it simpler while keeping devtools and hot reloading working, please do!

[gaearon]

(I don't mind breakage, we'll jump to 2.0 and that'll be it, but we need to make sure new API is better.)

[jlongster]

Here's a simple change that keeps the current interface for middlewares and higher-order stores. Make compose work like normal:

function compose(...funcs) {
  return (...args) => {
    const initialValue = funcs[funcs.length - 1].apply(null, args);
    const leftFuncs = funcs.slice(0, -1);
    return leftFuncs.reduceRight((composed, f) => f(composed),
                                 initialValue);
  };
}

i.e. it just composes foo, bar, and baz into (...args) => foo(bar(baz(...args))).

Middlewares would still return a function like next => action => ... and higher-order stores next => (reducers, initialValue) => .... But instead of writing:

compose(
  applyMiddleware(middleware1, middleware2),
  higherOrderStore,
  createDispatcher
)

You write:

compose(
  applyMiddleware(middleware1, middleware2),
  higherOrderStore
)(createDispatcher)

And in applyMiddleware the composition would be dispatch = compose(...chain)(dispatcher.dispatch). The only change is to only compose the functions actually being composed, and pass in the initial value.

[jlongster]

I haven't fully read through the other issues, sorry, particularly regarding removing replaceReducer. I can try to soon.

[gaearon]

Oh it makes sense now. (I assume you meant createStore of course.) cc @acdlite

[jlongster]

Hah, yes, I am using a simplified version of redux within the devtools until we can make things immutable...

[timdorr]

What about relying on an external compose implementation, like that from Lodash or Ramda? Since you're going to use it anyways

[gaearon]

Yes.

[gaearon]

Fixed by https://github.com/rackt/redux/pull/669. Since it's a breaking change, we'll release it in 2.0.

[gaearon]

Fixed in 2.0.

[acdlite]

Just ran into this... good to see it's already been fixed!

[Bjvanminnen]

It would seem that we still don't have the ability to seed our sequence with multiple arguments. Is this expected?

it('can be seeded with multiple arguments', () => {
      const square = x => x * x
      const add = (x, y) => x + y
      expect(compose(square, add)(1, 2)).toBe(9) // results in NaN instead
})

[gaearon]

@Bjvanminnen

I'm happy to accept a PR fixing this.

[nothingrealhappen]

compose still have problem redux@3.5.1

Cannot read property 'apply' of undefined

Test browser Version 9.1 (11601.5.17.1), Chrome Canary 52.0.2715.0

const store = createStore(reducers, {}, compose(
  applyMiddleware(ReduxThunk),
  window.devToolsExtension ? window.devToolsExtension() : undefined
));

replace with lodash reduceRight, everything works.

[timdorr]

You can't pass undefined. Pass a dummy function instead:

const store = createStore(reducers, {}, reduceRight(
  applyMiddleware(ReduxThunk),
  window.devToolsExtension ? window.devToolsExtension() : f => f
))

Or, if you like lodash, you can pass _.identity

[gaearon]

Maybe we should accept a PR that removes falsy values.