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. |
src/components/connect.js Outdated
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? |
4 participants
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.