React-redux: Render props version of connect

[cbranch101]

We've been trying to migrate a lot of our containers from HOCs over to components that use render props

For the sake of consistency we're doing the same for connect

import PropTypes from "prop-types"
import { connect } from "react-redux"

const ReduxDataHandler = ({ render, ...propsToPass }) => {
    return render(propsToPass)
}

ReduxDataHandler.propTypes = {
    render: PropTypes.func.isRequired
}

const withReduxData = (mapStateToProps, mapDispatchToProps) => {
    return connect(mapStateToProps, mapDispatchToProps)(ReduxDataHandler)
}

export default withReduxData

What would be awesome is if it would be possible to add a built in version of connect that does something similar but doesn't require adding this unnecessary wrapping. Would something like that be feasible?

[markerikson]

I don't think we have any plans to change the current API. There are a few existing re-implementations of connect as a render prop form, and this _was_ actually the original way connect worked - see https://github.com/jsonnull/redux-render , https://medium.com/@gott/connecting-react-component-to-redux-store-with-render-callback-53fd044bb42b , and https://twitter.com/threepointone/status/913701233394900992 .

@jimbolla , @timdorr : we don't actually export the various selectors that connect uses internally, do we? Can those be imported from "react-redux/lib/" right now? If so, that would let someone reuse that functionality for their own use case.

[estaub]

+1 . I expect you'll get a lot more over time.
FWIW, I use Typescript, and it really plays poorly with HOC, IMHO.

[corlaez]

My 2 cents on this. This can be 100% backwards compatible, just adding a diferent way to connect, nobody breaks and we are all happy. Why relying in custom implementation or yet-another-js-library just to connect with render props?

[B-Reif]

I've encountered some serious issues with typing around connect's HOC pattern and I want to lay some of them out here:

• To correctly type injected props, mapStateToProps needs its own type for its return value, say InjectedProps. That type duplicates the Props type expected by the connected component. This repetition causes fragmentation between InjectedProps and Props and doubles developer effort in typing.
• The container's type doesn't track changes in the connected component. If Props changes, mapState doesn't know unless InjectedProps is also updated. If you don't hand-change both, your app is broken, and Flow doesn't tell you.
• mapDispatch suffers from the same issues. Worse, when using the shorthand object signature for mapDispatch, typing does not work at all.
  
  
  
  • When Flow _is_ able to catch something, it often emits cryptic errors unrelated to the actual type mapping (unless it doesn't match the local InjectedProps type).
  
  

These are specific problems I've encountered while using connect in a flow-typed codebase. It sounds like TS codebases encounter similar issues. Maybe some problems could be solved through react-redux internals. But the path of least resistance--the library's core offering--should be easily type-compatible. Using typed Javascript is _not_ a niche use case.

A render prop would solve these issues. If, inside of the render prop function, I'm just rendering a regular ConnectedComponent, then everything going in is being typed by ConnectedComponent's own Props type.

In general, the pitfalls of HOCs have already been discussed at length elsewhere, so I won't go into detail on that topic. But I would add: code that's difficult for a type system to follow is probably difficult for humans to follow, too.

[markerikson]

@B-Reif : part of the issue is that none of us primary maintainers actually use TS or Flow, to my knowledge. Tim oversaw accepting PRs for TS updates for Redux 4.0, but we don't have the knowledge to actually work on any of the typings ourselves.

Beyond that... I get that typing is a big deal to a lot of people, but our core concern is a library that works as JS code. Redux and React-Redux have a lot of dynamic behavior, and I don't think there's any easy solutions for typing most of that.

[estaub]

@B-Reif I've settled on this pattern that works well for me:

import {compose} from 'recompose'

type IOwnProps = ... // define the outer props that aren't provided by HOCs

and then, whichever are needed, of:

type IReduxProps = ... // from connect
type StyleProps = ... // from Material-UI's withStyles HOC
type GraphqlProps = ... // from React-Apollo's graphql HOC

etc. and then

type IProps = IOwnProps & IReduxProps & ... // whichever are needed

class MyComponentView extends React.Component<IProps,{}> {
...
}

// create various HOC instances here using connect, graphql, withStyled, etc.

export MyComponent = 
     compose<IProps, IOwnProps>(connector, queryer, styler)(MyComponentView)

I don't claim this is wonderful, just very livable, and DRY. Some of the HOC instances may need template parameters, but usually not, because compose forces the types at each end of the HOC pipeline. Of course, that means that it may cover up mistakes that you've made, so where mistakes are likely, you probably want to explicitly apply type-params to the individual HOC instances (connect(), et al).

[B-Reif]

@markerikson I only just learned elsewhere that you're open to render props in v6 so as long as that's on the table I have no problem 😄

@estaub This is pretty good! Certainly not ideal but way better than repetition.

[JesseChrestler]

Is there a way we can take a step back from the API? maybe simplify it to be more terse and and expressive.
First of all I would get rid of mapDispatchToProps, not only does it encapsulate your logic (which makes it harder to follow) but it can be provided separately. Maybe the API could look more like this?

const example = () => {
  return (
    <Connect user={userSelector.getUser}>
      {({ user, dispatch }) => {
        return (
          <div>
            {user.firstName} {user.lastName}
          </div>
        );
      }}
    </Connect>
  );
};

Just trying to shake things up, and maybe reshape the way we write our connect code.

[markerikson]

@JesseChrestler : as discussed in our roadmap in #950 , the plan is to keep the current API as-is for v6, and then open things up to discussion for possible changes in v7.

The point of mapDispatch _is_ to encapsulate logic, so that both the wrapped component and any descendant components are simply dealing with functions. For example, if you want to dispatch an action when a button is clicked, you don't want that button itself to be aware that it's "dispatching" something. You just want <button onClick={doSomething}>. In order for that to happen, you need a wrapper function that captures dispatch and dispatches the appropriate action when the function is executed. That's what mapDispatch is intended for.

Sure, you can totally do it by hand inside your component, or by writing an actual mapDispatch function, but why do that by hand every time when you can pass an object full of action creators to connect and let it do that for you?

I'd encourage you to go back and read through issue #1 , where Dan laid out the desired constraints for the React-Redux API.

[JesseChrestler]

@markerikson I really like your point about mapDispatch. You've got me thinking more, which I appreciate. I will definitely look at issue #1 and more thinking on this. I don't typically like how bindActionCreators takes the developer out of the typical flow and you can't easily tell where things are coming from, however with dispatch you can. Pros and Cons, I get it. Thanks again for the quick response.

[markerikson]

I would generally argue that a "good" React component shouldn't care _where_ its props come from. It should just get data and functions as props, and when it needs to kick off some external behavior, it just calls this.props.doTheThing() - it shouldn't care whether doTheThing is a bound Redux action creator, a callback from a parent, or a mock method in a test. So, in the case of a Redux-connected component, A) we want to wrap up the action creators so that they're "just" props, B) it does make your own components more generic, and C) since most of the time we just want to immediately call a given action creator with the provided arguments and call dispatch() with the results, there's no reason to call bindActionCreators yourself - just let connect do it for you.

[kylecorbelli]

import * as React from 'react'
import { connect } from 'react-redux'
import { Dispatch } from 'redux'
import { ReduxAction } from '../store/actions'
import { ReduxState } from '../types'

interface ReduxProps extends Readonly<{
  dispatch: Dispatch<ReduxAction>
  state: ReduxState
}> {}

type Props = ReduxProps & Readonly<{
  children: ({ dispatch, state }: ReduxProps) => JSX.Element | null
}>

const ConnectView: React.FunctionComponent<Props> = ({
  children,
  dispatch,
  state,
}) =>
  children({
    dispatch,
    state,
  })

const mapStateToProps = (state: ReduxState) => ({ state })

const mapDispatchToProps = (dispatch: Dispatch<ReduxAction>) => ({ dispatch })

export const Connect = connect(
  mapStateToProps,
  mapDispatchToProps,
)(ConnectView)

would let us write the following example:

import * as React from 'react'
import { Connect } from '../Connect'
import { Decrement, Increment } from '../store/actions'
import { Quantity as QuantityView } from './view'

export const Quantity = () =>
  <Connect>
    {({ state, dispatch }) => (
      <QuantityView
        count={state.count}
        decrement={() => dispatch(Decrement())}
        increment={() => dispatch(Increment())}
      />
    )}
  </Connect>

without ever needing a mapStateToProps or mapDispatchToProps again.

[markerikson]

@kylecorbelli : if you do that, your component would now re-render for _every_ dispatched action that resulted in a state change. Also, you're having to dispatch() by hand. (For that matter, if you want dispatch as a prop, you don't have to pass it down manually - just don't supply the second argument to connect and you'll get it as a prop automatically.)

You can do that, if you want to, but it's not efficient at all.

You might want to review the design constraints listed in issue #1 to understand why the API works this way. (I'm also writing a blog post on this topic, which I hope to have done later this week.)

[kylecorbelli]

Yeah, that's the real sticking point isn't it? 😭

[kylecorbelli]

import * as React from 'react'
import { connect } from 'react-redux'
import { Dispatch } from 'redux'
import { ReduxAction } from '../store/actions'
import { ReduxState } from '../types'

type MapStateToProps<P> = (state: ReduxState) => P

type MapDispatchToProps<P> = (dispatch: Dispatch<ReduxAction>) => P

interface ConnectProps<StateProps, DispatchProps> extends Readonly<{
  children: (props: StateProps & DispatchProps) => JSX.Element | null
  mapDispatchToProps: MapDispatchToProps<DispatchProps>
  mapStateToProps: MapStateToProps<StateProps>
}> {}

export class Connect<StateProps, DispatchProps> extends React.Component<ConnectProps<StateProps, DispatchProps>> {
  public render () {
    const { children, mapStateToProps, mapDispatchToProps } = this.props
    const Component = connect(
      mapStateToProps,
      mapDispatchToProps,
    )(props => children(props))
    return <Component />
  }
}

will allow us to write:

import * as React from 'react'
import { Dispatch } from 'redux'
import { Connect } from '../Connect'
import { Increment, ReduxAction } from '../store/actions'
import { ReduxState } from '../types'

const mapStateToProps = (state: ReduxState) => ({
  count: state.count,
})

const mapDispatchToProps = (dispatch: Dispatch<ReduxAction>) => ({
  increment: () => dispatch(Increment()),
})

export const Quantity: React.FunctionComponent = () =>
  <Connect mapStateToProps={mapStateToProps} mapDispatchToProps={mapDispatchToProps}>
    {({ count, increment }) => (
      <div>
        <p>{count}</p>
        <button onClick={increment}>Increment</button>
      </div>
    )}
  </Connect>

and through the use of generics we even get type safety on the props passed to the children function of the render prop component.

This should prevent re-rendering when unrelated state changes occur.

[markerikson]

That example is broken, because you're calling connect() every time the render prop component re-renders. That will always return a new component type, and thus cause React to throw away the entire existing component tree.

Yes, render props versions of connect are entirely possible. A bunch of them exist already from the community. We're just not ready to add one to our official API. (And, given the existence of hooks, I'm starting to think that we may not actually do so in the long run.)

[kylecorbelli]

¯\_(ツ)_/¯