Remove filtering inside compose #2167

Merged
merged 1 commit into from Dec 30, 2016

Projects

None yet

5 participants

@istarkov
Contributor
istarkov commented Dec 24, 2016 edited
  1. With filtering compose does not have the well know definition of this function as like as in your comments https://github.com/reactjs/redux/blob/master/src/compose.js#L7-L9
  2. Then I accidentally pass something what is not a function into compose, the only thing I could expect is an error not silent code execution. (there is no need at all to pass anything which is not a function)
  3. Typed definition of compose (even you does not use it) will be really hard to create with filtering.

So I suggest you to remove this line.

@chicoxyzzy
Contributor
chicoxyzzy commented Dec 24, 2016 edited

As a developer I expect my code to throw an Error, not eat it. Also I strongly opposite to extra useless computations in redux. Why not to filter functions outside of compose if you really need this?
So big 👍 to this PR

@alexeyraspopov

This breaks the @evilj0e's use case described in #2073

@alexeyraspopov

But throwing errors explicitly is definitely a better thing.

@jimbolla
Member

What if it were to only filter out falsey?

@chicoxyzzy
Contributor
chicoxyzzy commented Dec 24, 2016 edited

@jimbolla why filter something at all?
The problem described in #2073 can be solved in many ways by one-liner. But the fix from #2073 is a fix of a particular codebase problems and it leads to ommited errors and performance regressions in others people code

@chicoxyzzy
Contributor

One can filter everything he wants to outside of compose. It's so easy I can't even

@timdorr
Member
timdorr commented Dec 30, 2016

Good points being made. Thanks!

@timdorr timdorr merged commit b4fb081 into reactjs:master Dec 30, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@gaearon gaearon changed the title from Remove filtering inside compose to Remove filtering inside compose Dec 30, 2016
@istarkov istarkov referenced this pull request in acdlite/recompose Dec 30, 2016
Closed

Added in non-function filtering and test case #292

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment