search

FirebaseExtended / reactfire

Hooks, Context Providers, and Components that make it easy to interact with Firebase.
https://firebaseopensource.com/projects/firebaseextended/reactfire/
MIT License
3.5k stars 400 forks source link

Provide a way to bind to a props-dependent ref #66

Closed jyasskin closed 6 years ago

jyasskin commented 8 years ago

This has come up in a couple questions (#31, #49, http://stackoverflow.com/q/32688547/943619), so I'm going to attempt to propose an API change to fix it. I'd be totally happy with another solution, of course.

I want to bind a dynamic Firebase reference to this.state.foo. We want to recompute the reference whenever props change (*). If the reference hasn't changed, we don't want any extra network traffic, as seems to happen when fbref.off(); fbref.on() are called. If the reference has changed, it'd be nice to avoid the flicker from an intermediate this.setState({foo: undefined}) as happens in unbind().

So, bindAsObject() and bindAsArray() should accept a function (props)=>ref as their first parameter, in addition to the existing Firebase ref. (That is, bindAsX(functionOrRef, name, callback).) This should be called at bind time with this.props, and again in componentWillReceiveProps. If it returns the same ref as the previous call, nothing should happen. If it returns a new ref, ReactFire should unbind from the old reference and bind to the new one. ReactFire should not set the state to undefined while the new data loads, to avoid making the UI jump twice. Optionally, ReactFire should set a "loading" state of some sort so the UI can grey-out or otherwise disable the right regions of the component while it's out of date.

(*) In general, the reference could be dynamic based on both state (especially auth data from #65) and props, but restricting it to props makes sure data only flows in one direction, and state can always(?) be turned into props by nesting an extra level.

jwngr commented 8 years ago

Thanks for the suggestion (and apologies on not responding earlier)! I think this is a good suggestion. I unfortunately just don't have a ton of time to look into it right now. I would happily code review a PR if you have time to put one together. Otherwise, I'll get to this once I have some more time to devote to the growing ReactFire issues list.

jyasskin commented 8 years ago

Just to set expectations, it's unlikely I'll find time to do a PR.

jwngr commented 8 years ago

Sounds like we have something in common :ghost: Hopefully I'll free up some time soon though.

samtstern commented 6 years ago

We have officially deprecated reactfire, as we no longer believe it is the best solution for Firebase developers looking to integrate with React.

Please see the README for information on how to migrate to a better solution. We are working on some new efforts to better serve the React community in the future.