Recompose: "withHandlers" recreates functions after each component update?

Created on 10 Jun 2016  Â·  4Comments  Â·  Source: acdlite/recompose

It is easy to see right in your basic fiddle: https://jsfiddle.net/au1y21hL/2/

Although withHandlers really is "shouldComponentUpdate-friendly", this statement:

This avoids a common pitfall where functional components create handlers inside the body of the function, which results in a new handler on every render

is true only if handler does not cause a parent's update – while in most cases it actually does. Even in such a simple case as counter.

Is it worth trying to optimize this moment?
For example, by passing props directly to handler (as first param) instead of using handler creator. Or some another solution?

docs

Most helpful comment

Yeah that line is a bit misleading. You still have to re-create the handler every time the props change.

For example, by passing props directly to handler (as first param) instead of using handler creator.

This would have the same problem. At some point, you'd have to do this:

handler = (...args) => handlerCreator(this.props, ...args)

I'm not sure if this is really that big of a deal. We should update the docs to make it clear that the purpose of withHandlers is to provide immutable function references, not to prevent function allocations.

All 4 comments

Yeah that line is a bit misleading. You still have to re-create the handler every time the props change.

For example, by passing props directly to handler (as first param) instead of using handler creator.

This would have the same problem. At some point, you'd have to do this:

handler = (...args) => handlerCreator(this.props, ...args)

I'm not sure if this is really that big of a deal. We should update the docs to make it clear that the purpose of withHandlers is to provide immutable function references, not to prevent function allocations.

Ugly Variant: We can create proxy object, so there will be no reason to drop cachedHandlers, but just to update this proxy object. This will slightly change API to have reference to proxy https://github.com/Coobaha/recompose/commit/202a7db7bd28c73a51c8351bf3f07297a043e171.

Another variant is to drop handlers cache creation logic, and create corresponding instance method, which will create handler with current props and call it with the rest args.
Plus we can drop handler creation part, and just change handler signature to accept props as first param, which will simplify API.

If something of this fits, let me know, I will create a PR.

Thanks for your advice!

Quick response to the first approach: the props in the proxy object is mutable across handlers, which might lead to unexpected behaviors.

For example

withHandlers({
  foo: proxy => () => proxy.props = { bar: 123 }
  // After foo has been invoked, all other handlers would receive the mutated props
})

Worth noting I slightly misspoke above. The handlers are created lazily, only when they are called. So re-rendering does not cause a handler to be created until/unless it is called. And if you call the handler again before a re-render, the handler is cached.

Long story short, I don't think there's any performance problem with the current implementation, and I'm loathe to consider alternatives unless someone can produce a benchmark that shows otherwise.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

cdomigan picture cdomigan  Â·  4Comments

uriklar picture uriklar  Â·  4Comments

xialvjun picture xialvjun  Â·  4Comments

rndmerle picture rndmerle  Â·  3Comments

robbporto picture robbporto  Â·  3Comments