Overloading connect with factory #279
Conversation
|
Rather than look at arg count, can we determine whether they returned functions instead of objects lazily first time we call them from the constructor? |
|
Took another pass at it, it's a little more verbose / complicated, but does things lazily as you mentioned, all tests are passing and it avoids calling the mapping functions any more times than necessary. If this looks alright I can add some tests. |
| @@ -14,65 +14,40 @@ const defaultMergeProps = (stateProps, dispatchProps, parentProps) => ({ | |||
| ...dispatchProps | |||
| }) | |||
|
|
|||
| function isFunction(fn) { | |||
There was a problem hiding this comment.
I would leave those checks inline.
|
Yes, this looks good. Please do add tests. |
tgriesser
force-pushed
the
mapStateToFactory
branch
from
55995db
to
43344c9
Compare
|
Updated, rebased, tests added. Also noticed when writing the tests that a final shallow equality check on the final merged props would save a render if there is a custom |
tgriesser
force-pushed
the
mapStateToFactory
branch
from
06de7fd
to
4bb0087
Compare
| expect(updatedCount).toBe(0) | ||
| expect(memoizedReturnCount).toBe(2) | ||
| }) | ||
|
|
There was a problem hiding this comment.
Nit: mind killing the newline here?
|
That’s amazing, thank you so much for your work on this. A few more nits and it’s good to go. |
|
All set with a test and a bit of formatting polish |
Overloading connect with factory
|
Would you be up to contributing a section to Computing Derived Data recipe documenting that? I think a lot of people won’t learn about this useful feature otherwise. |
|
Sure thing, I'll take a look at doing this a little later. |
|
@tgriesser I have started to update the Reselect docs to reflect this change. If you haven't started on this already, would you like me to carry on and document it? |
I had actually considered this before doing the
connectFactoryapproach in #185. The issue was that it felt wrong to overload the arguments and require first calling the function to determine its return type.That said, this is the cleanest implementation I could come up with. It checks if the argument in question has
fn.lengthof 0 and also returns a function.This breaks a few tests, but nothing critical - only on some spies or guarantees an arity-0 function is called a certain amount of times. I can clean those up if this looks alright otherwise.