reduxjs / redux

A JS library for predictable global state management
https://redux.js.org
MIT License
60.88k stars 15.27k forks source link

Controversial Alert: React Router 4 style Redux #1968

Closed jfrolich closed 7 years ago

jfrolich commented 8 years ago

Hi all,

I was reading through the React Router 4 alpha docs. Building an API with components, even when it is not really an UI element really struck a chord with me. Do components really need to be UI and be idempotent?

The new React Router API is so simple, and intuitive, because everyone that uses react knows how to use components. You can also leverage the niceties of react like the lifecycle methods. It does however seem to be very controversial in the community.

I was thinking other use cases where this approach might works. So I looked at redux. Some of the API can be confusing for beginners, especially the connect HOC api.

Why not wrap our dumb components in a <Connect> component that provides redux state to its child(ren).

This would come down to the following API:

const NavBar = ({appName}) => <div>{appName}</div>

const NavbarContainer = () => (
  <Connect mapStateToProps={(state) => state.appName}>
    <Navbar />
  </Connect>
)

We can also even think further. Perhaps action creators as components that send an action on mount or unmount. Obviously we still need the old API for side-effects outside components, but it might make sense when we want to fire an action on mount and unmount to handle data fetches or subscriptions (that are picked up by a saga or other side effect). Maybe it would make sense to comopose reducers using components?

I did not send a push request yet, because I am not sure if this is a good idea, maybe the pendulum is swinging too far, and this is just nuts. Anyway I am experimenting with this in my own app, and I would like to bring this up for discussion, and see what you guys think of this. Below the (hacky) code that provides the <Connect/> API:

import React from 'react';
import { connect } from 'react-redux';

const Connect = ({
      mapStateToProps,
      mapDispatchToProps,
      mergeProps,
      options,
      children,
      ...rest
    }) => React.createElement(
      connect(
        mapStateToProps,
        mapDispatchToProps,
        mergeProps,
        options)(
          ({ children, ...rest }) => {
            return <div>{React.Children.map(children,
                 (child) => React.cloneElement(
                  child,
                  rest)
                )}</div>
          }
        ),
        rest, children
      );

export default Connect;

(I couldn't find a way to remove the div tag.) Have fun! And let me know what you think!

gaearon commented 8 years ago

For the record, <Connect> was the original API for React Redux but it didn’t have good performance for various reasons. React Router can get away with it because it doesn’t deal with global subscriptions that fire often and have effect on most components in the app.

https://github.com/reactjs/react-redux/issues/1

gaearon commented 8 years ago

Maybe it would make sense to comopose reducers using components?

You sort of can:

const reducer = ...

class MyComponent extends Component {
  dispatch(action) {
    this.setState(prevState => reducer(prevState, action))
  }

  render() {
    // ...
  }
}

Many people take Redux too seriously and don’t realize you can mix and match it with React state, or use reducers with component local state.

But I’m open to see more experiments!

jfrolich commented 8 years ago

Wow so quick. Thanks for the response. Is the main reason this is not performing well that you have to inject the props in the children? Because otherwise would there be really a difference vs a higher order component?

gaearon commented 8 years ago

I believe the linked issue contains some of the relevant discussion about this, as well as a list of criteria we were using at the time.

jfrolich commented 8 years ago

It's a very extensive issue, so for the ones interested, this specific comment seems to address it: https://github.com/reactjs/react-redux/issues/1#issuecomment-120719430. The performance bottleneck seems to be the rebinding of functions on each render.

I am not sure if this can be fixed by a pre-defined selector function. Will look into this later.

Thanks for the feedback @gaearon, and taking the time to comment on this, was very helpful! Might post some more experiments in this thread.

timdorr commented 7 years ago

@jfrolich If you put anything together, feel free to open up a PR on the react-redux repo!