Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create ReduxConnection component #920

Closed
Sawtaytoes opened this issue Apr 4, 2018 · 9 comments
Closed

Create ReduxConnection component #920

Sawtaytoes opened this issue Apr 4, 2018 · 9 comments

Comments

@Sawtaytoes
Copy link

Sawtaytoes commented Apr 4, 2018

I'd like to get away from the HoC connect method and instead utilize render props. While higher-order component functions are great if you're composing components in an export, I'd rather use JSX components to compose components instead. This allows for more freedom in how components are rendered and allows for the creation of dumb components that can dynamically listen to various Redux state props as-needed.

I've been working on my own component to do just that called ReduxConnection, but it's one of those things that probably should be maintained by the react-redux project. Digging through the source code, I found something similar, a Connect component that exists in connectAdvanced. If that was exposed somehow, wouldn't it allow for the same concept I'm referencing?

This is the current ReduxConnection component as I've got it:

const ReduxConnection = ({
  children,
  component,
  mapDispatchToProps = null,
  mapStateToProps = null,
  render,
  ...props
}) => (
  React.createElement(
    connect(mapStateToProps, mapDispatchToProps)(
      (component || render || children || throwError)
    ),
    props
  )
)

Unlike React Router's Route implementation, I can't manage the different rendering methods for component, render, or children. I personally like the distinction used by React Router and think it's a good template for other components with render props. This particular ReduxConnection component sends components to the connect HoC regardless of how those props should be handled.

The worst part is I have to be explicit in which props I'm accepting. Right now, I've only got it setup with mapStateToProps and mapStateToDispatch, but theoretically, it should support all function methods.

An example usage where this would be beneficial:

// `component`
<ReduxConnection
  namespace="someNamespace"
  mapStateToProps={loadingStateSelector}
  component={LoadingIndicator}
/>

// `render`
<ReduxConnection
  namespace="someOtherNamespace"
  mapStateToProps={loadingStateSelector}
  render={({ isLoading }) => (
    <LoadingIndicator isLoading={isLoading}>
      <div>Some Content</div>
    </LoadingIndicator>
  )}
/>

I can put these two components side-by-side or in the same parent component without the parent component needing to know any of its children's state props. This also allows LoadingIndicator to be stateless, and only stateful when required.

The ReduxConnection component also solves this issue of exporting both the stateless, unit-testable component and the connect-wrapped version like so:

// I dislike doing this but need to for unit testing.
export LoadingIndicator () => <div />
export default connect(state => state)(LoadingIndicator)

While this doesn't solve every unit-testing use case, it explicitly defines which props components are actually receiving versus ones they're grabbing from the ReduxConnection render prop. With connect, you have to mix in a bunch of extra props in your PropTypes which connect is passing in.

Another benefit is the ability to compose it like this:

const LoadingState = props => (
  <ReduxConnection
    {...props}
    mapStateToProps={loadingStateSelector}
  />
)

const LoadingInformation = () => (
  <Fragment>
    <LoadingState
      component={LoadingIndicator}
      namespace="someNamespace"
    />
    <LoadingState
      component={SpecialLoadingIndicator}
      namespace="someOtherNamespace"
    />
  </Fragment>
)

Lastly, if you've used React Hot Loader, you've probably run into higher-order component hot-reload issues before. The way I've gotten around that is using the extract-hoc/babel plugin in my Babel plugins. Not ideal. It's a lot easier to just use a React component and bypass this hack entirely.

Have I shown enough of a case for the addition of a render props version of connect or are there still unanswered questions?

References:

@timdorr
Copy link
Member

timdorr commented Apr 4, 2018

This is something we're discussing along with the async changes. Check out #890

@timdorr timdorr closed this as completed Apr 4, 2018
@Sawtaytoes
Copy link
Author

Sawtaytoes commented Apr 9, 2018

I think my request is different. I don't want to use an HoC for connect; I want it to be a component instead and use render props in the same way React Router does it.

Could you explain how #890 is related?

@gnapse
Copy link

gnapse commented Apr 9, 2018

I too do not know how #890 is related, but this is anyway a duplicate of #799. Perhaps checking out the discussion there you'd find out what about this feature request.

@markerikson
Copy link
Contributor

We're not planning to implement a render-props-based API for v5. However, since we'll likely have to reconsider the React-Redux API as a whole for v6 to fully take advantage of async rendering, a render props API may be a possibility at that time.

@marcelmokos
Copy link

Do you have an estimation when the React-Redux v6 will be released?
We want to use render prop connector component abstraction as soon as possible in the project.

@timdorr
Copy link
Member

timdorr commented Apr 13, 2018

@marcelmokos No time soon. Just use that pattern in your code right now. It's great.

@marcelmokos
Copy link

marcelmokos commented Apr 13, 2018

@timdorr the biggest issue is to be able to correctly flow-type the Connect and to not lost the flow-types used in redux.

// @flow
import * as React from "react";
import {connect} from "react-redux";

type Props = {
  /** Render prop. */
  children?: React.Element<*>,
  /** React component */
  component?: React.Element<*>,
  /** redux mapStateToProps */
  mapStateToProps?: Function,
  /** redux mapStateToProps */
  mapDispatchToProps?: Object,
  /** render */
  render?: Function,
};

const Nothing = () => null;

const Connect = ({
  children,
  component,
  mapDispatchToProps = {},
  mapStateToProps = (state, ownProps) => ownProps,
  render,
  ...props
}: Props) =>
  React.createElement(
    connect(mapStateToProps, mapDispatchToProps)(
      component || render || children || Nothing,
    ),
    props,
  );

export default Connect;

And tests

// @flow
import React from "react";
import {mount} from "enzyme";
import Connect from "./Connect";
import StoreSingleton from "shared-store";
import {Provider} from "react-redux";

const store = StoreSingleton.getStore();

describe("<WithState />", () => {
  const Component = props => <div />;

  const actionCreator = (type: string) => (payload: Object) => ({
    type,
    payload,
  });

  const actionA = actionCreator("A");
  const actionB = actionCreator("B");

  it("mount without props", () => {
    const wrapper = mount(
      <Provider store={store}>
        <Connect>
          {props => <Component {...props} ownProp="myOwnProp" />}
        </Connect>
      </Provider>,
      {context: store},
    );
    expect(wrapper.find(Component).props()).toMatchSnapshot();
  });

  it("mount with mapStateToProps and mapDispatchToProps", () => {
    const wrapper = mount(
      <Provider store={store}>
        <Connect
          mapStateToProps={state => ({
            state,
          })}
          mapDispatchToProps={{actionA}}
        >
          {props => <Component {...props} ownProp="myOwnProp" />}
        </Connect>
      </Provider>,
      {context: store},
    );
    expect(wrapper.find(Component).props()).toMatchSnapshot();
  });

  it("mount with mapDispatchToProps", () => {
    const wrapper = mount(
      <Provider store={store}>
        <Connect mapDispatchToProps={{actionA, actionB}}>
          {props => <Component {...props} ownProp="myOwnProp" />}
        </Connect>
      </Provider>,
      {context: store},
    );
    expect(wrapper.find(Component).props()).toMatchSnapshot();
  });

  it("mount with mapDispatchToProps with ownProp override", () => {
    const wrapper = mount(
      <Provider store={store}>
        <Connect mapDispatchToProps={{actionA, actionB}}>
          {props => <Component {...props} actionB="myOwnAction" />}
        </Connect>
      </Provider>,
      {context: store},
    );
    expect(wrapper.find(Component).props()).toMatchSnapshot();
  });
});

And snapshots

// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`<WithState /> mount with mapDispatchToProps 1`] = `
Object {
  "actionA": [Function],
  "actionB": [Function],
  "ownProp": "myOwnProp",
}
`;

exports[`<WithState /> mount with mapDispatchToProps with ownProp override 1`] = `
Object {
  "actionA": [Function],
  "actionB": "myOwnAction",
}
`;

exports[`<WithState /> mount with mapStateToProps and mapDispatchToProps 1`] = `
Object {
  "actionA": [Function],
  "ownProp": "myOwnProp",
  "state": Immutable.Map {
    "route": Immutable.Map {
      "location": null,
    },
    "form": Immutable.Map {},
  },
}
`;

exports[`<WithState /> mount without props 1`] = `
Object {
  "ownProp": "myOwnProp",
}
`;

@Sawtaytoes
Copy link
Author

I'd completely forgotten about this issue and know it got consolidated elsewhere, but I wanted to update it with my own implementation of my original question.

This could be used as an example of how to do it because I would definitely want to update its API to be more similar to the new Context API if something like this was going to be in react-redux.

ReduxConnection component:
https://github.com/Sawtaytoes/Ghadyani-Framework-Redux-Components/blob/master/components/ReduxConnection.js

Article on how to use it:
https://medium.com/@Sawtaytoes/why-you-shouldnt-need-connect-from-react-redux-498876de9e4e

@markerikson
Copy link
Contributor

@Sawtaytoes : see my latest thoughts on this topic over in #799 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants