Decorators should not mutate existing classes

[adidahiya]

Following up on #925 (comment)

Higher order components should wrap existing components, not mutate their prototype directly. Check out the pattern documented in react-bits.

Thinking about this also makes me hesitant about implementing #724 since that situation is similar. It's a little different because that one deals with the real DOM rather than the component tree. There, we are proposing switching from an element composition approach to one where we more directly mutate child elements, which sounds antithetical to the direction I'd like to go with decorators in the public API.

---

List of components to fix:

• HotkeysTarget
• ContextMenuTarget

[giladgray]

i've been looking into this and it's much more challenging to implement these decorators without mutating. for instance, HotkeysTarget replaces the render method and makes great use of the fact that it's called within the component instance so it can easily invoke renderHotkeys. without mutation, this requires a ref to get the instance and a lot more lifecycle juggling to make sure things happen at the right time.

make me wonder if the hotkeys API is truly the best API for the feature in React?

also typings for HOCs are very obnoxious cuz you want the HOC to be a transparent outer layer, but i have not yet figured out how to express this in TS...

[adidahiya]

Some references for typing higher order components:

• react-redux @connect
• react-dnd decorators

There's probably also some useful JS reference there on whether you need to get a ref and what you need to do with the lifecycle. Getting a component ref doesn't sound so bad to me (you're not doing any DOM manipulation). What lifecycle juggling is required?