Added in non-function filtering and test case

[JoeCortopassi]

current implementation breaks if >1 arguments are given, but none are functions. There is already a test case for one argument, this just enhances that

[istarkov]

Hi, Thank you.
Why should compose not to throw on non function arguments?

[JoeCortopassi]

@istarkov in a functional flow, not all inputs to a composition are always known. It's the same reason it gives back an identity function for a no argument case

[istarkov]

not all inputs to a composition are always known

Can't get, why?

[JoeCortopassi]

@istarkov simple example of what I'm talking about

compose(
  isDev ? someEnhancer : null,
  isDev ? anotherEnhancer : null
)

[JoeCortopassi]

also, with the current implementation, if one argument was passed and it wasn't a function, the composer would return it back thinking it was a valid function. The users code would then break later down the stack, leading to some confusing errors

[istarkov]

I'm not agree with this.

The reasons are:
recompose can use any compose function, for example from underscore or rambda,
just because it uses usual definition of compose function: compose(a,b,c)(arg) === a(b(c(arg)))
There are no any magic or filtering well known here. For example what ramda will say you example if you will try to use anything that's not a function.

Type declaration of compose (and people use flowtype, typescript) are always use that fact that arguments are functions, and even that functions are composable! please have a look at example, this PR will broke type declaration and make impossible to use compose with typed structure.

BTW in your case you can simply write in your code if you want

compose(...[
  isDev ? someEnhancer : null,
  isDev ? anotherEnhancer : null
 ].filter(isFunction)
)

It will not add any additional overhead to your code.

It's really good that you create a PR,
And I really don't want to tell that we can't accept it, but see the reasons above.

[JoeCortopassi]

@istarkov i'm not understanding at all how this will break type declaration. It does nothing to effect that.

If you want it to throw an error, rather than filtering, I can change it to that so it matches Ramda. But as it stands, compose does not error when given non functions. It just passes down the broken composition.

[istarkov]

Flowtype/typescript checks compose declaration and will give you an error about that null or anything else is incompatible with function type. So static type checkers will solve the need of checking every user input s/he could provide.
As you could see rambda also doesn't do variables checking at initiating and fails at call.

Having that modern static type checkers can solve this, IMO it's not a good idea to typecheck every user input at runtime even in dev mode.

[toddw]

@istarkov IMHO relying on typechecking is a bad idea. These tools are helpful but not everyone who uses recompose uses Flow or Typescript. Furthermore, it's entirely possible that the arguments passed to this method could be created dynamically at runtime. When that happens, typechecking wont be able to save you there.

[istarkov]

@toddw you suggest to add type checks to every enhancer and to every exposed function from the library?
This will make the source looks dirty vs sometimes some user will get a runtime error.
I can't say for all, but type errors is the most rare errors I have in my life.

And even javascript is not strongly typed language do we need to write all that checks in every function of our code? Why just compose?

I will not close this as no runtime type checks is just my code writing preferences can't say for others.

[istarkov]

Thread about reduxjs/redux#2145 for some reasons guys from redux accepted this.

[toddw]

type errors is the most rare errors I have in my life.

You're lucky man :) I wish I could say the same, that's why I find Flow so useful

I'm just suggesting that there is value to filtering for functions vs relying on static typing addons.

reduxjs/redux#2145

I was going to mention redux but you beat to the punch! :)

[istarkov]

In Russia if two mans saying or writing similar things simultaneously is a very bad sign ;-)

[istarkov]

Having that my proposal to remove filtering from compose at redux is accepted reduxjs/redux#2167 (comment)
And it based on reasons I wrote above, I'll close this.