altjs / utils

A collection of utils for alt.js
37 stars 25 forks source link

Add getRef() method to connectToStores to expose wrapped component #20

Closed lsanwick closed 6 years ago

lsanwick commented 8 years ago

In the case of exposed component functions, connectToStores lacks any way to provide access to them after wrapping the component. Rather than hoisting static/non-static functions, or something else that would be prone to errors if React/Alt internals change in the future, I just provide a convenience method on the wrapped component that will hand over the ref of the Component.

kellyrmilligan commented 8 years ago

I like this approach as well. I would counter that in the page linked above, that they mention this is/should be uncommon.

I do think however that hoisting a static function up to the returned component is a completely valid option as in #19

similarly to this: https://github.com/reactjs/react-redux/blob/ea53d6fb076359a864a1d543568d951d4b3eab3d/src/components/connect.js#L365

and see this discussion: https://github.com/reactjs/react-redux/issues/276

lsanwick commented 8 years ago

True, I think statics are an easier argument to hoist, as there are basically no built-in statics that you need to be concerned about. When looking at this I found hoist-non-react-methods, which does non-statics as well, but that seems prone to error in the long run, so I felt this was a cleaner solution that still exposed the functionality that someone might need.

kellyrmilligan commented 8 years ago

I guess it's in @goatslacker's hands. I want to similarly as discussed in other issues collect methods from matched routes and fire off the requests.

taion commented 8 years ago

You can't do this unconditionally. This will throw for functional components.