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

Connect could listen to the used state automatically #901

Closed
SkoomaCowboy opened this issue Mar 14, 2018 · 5 comments
Closed

Connect could listen to the used state automatically #901

SkoomaCowboy opened this issue Mar 14, 2018 · 5 comments

Comments

@SkoomaCowboy
Copy link

Hey, I was trying to reduce the redux boilerplate in my project and came up with a connect that instead of requiring mapStateToProps, automatically detects the state that was used in render. https://github.com/SkoomaCowboy/connect-redux Basically:

import { autoConnect } from "react-redux";

const ComponentName = (redux, props, context) => <div>{redux.count}</div>;

// no more mapStateToProps, listens only to the state that was used in the component
export default autoConnect(ComponentName);

It was more or less inspired by @acdlite https://twitter.com/acdlite/status/971598256454098944

I know it's a fat chance that you would consider expanding the API with autoConnect or changing the default connect behaviour, but I see very few drawbacks to this approach, so I had to ask. Feel free to close this issue :)

@markerikson
Copy link
Contributor

Definitely not planning to replace mapState or mapDispatch (and I absolutely disagree with the statement in that README that they're "useless").

However, the idea of detecting which state keys a connected component accesses is actually something I am interested in . Please see the discussion in #898 .

@SkoomaCowboy
Copy link
Author

Useless might be too aggressive of a word, but in a FP sense this:

import { connect } from "react-redux";

const mapState = (state) => ({
    count: state.count
})

const ComponentName = (props, context) => <div>{props.count}</div>;

export default connect(mapState)(ComponentName);

Is identical to:

import { autoConnect } from "react-redux";

const MapperComponent (redux, props, context) => { 
  return <ComponentName count={redux.count}/>;
}

const ComponentName = (props, context) => { 
  return <div>{props.count}</div>;
}

export default autoConnect(MapperComponent);

As you can see MapperComponent is useless because I could just beta reduce to

import { autoConnect } from "react-redux";

const ComponentName = (redux, props, context) => <div>{redux.count}</div>;

export default autoConnect(ComponentName);

This makes redux state clearly visible, adding a new state to a component creates less boilerplate, reading code requires less jumps and it works better in more complex cases (no mergeProps). I am quite happy with the result in our project, but I perfectly understand why you wouldn't want to change react-redux.

@markerikson
Copy link
Contributor

Redux generally encourages being explicit about what your code is doing. As part of that, the React-Redux API is designed so that you decide what exactly what pieces of data your component needs to extract from the Redux state, and connect will only re-render the component once those extracted values have changed.

Also, looking at your examples, they don't show how it works with derived values (such as mapping a list of user entries to collect a "full name" result based on the "first name" and "last name" fields.

@SkoomaCowboy
Copy link
Author

AutoConnection doesn't make code less explicit, I declare exactly the same thing just in a different place.

The current connect created unnecessary busywork for us:

  • finding what comes from a parent as a prop and what comes from redux state
  • using a new state changes 2 parts (mapState and render) thus making code reviews harder.
  • You need to jump between mapState and render when trying to understand a new component. I am used to reading my text top to bottom.

As you can see, none of my goals are adding more magic or making redux less explicit, just rearranging code to reduce boilerplate :)

Also, looking at your examples, they don't show how it works with derived values (such as mapping a list of user entries to collect a "full name" result based on the "first name" and "last name" fields.

I do have one in README:

import connect from "connect-redux";
import { Increment } from "./actions";
import { getFullName} from "./selectors";

const ComponentName = (redux, props, context) => {
  // same as mapStateToProps:
  const count = redux.count;
  const fullName = getFullName(redux)

  // same as mapDispatchToProps:
  const onClick = e => redux.dispatch(Increment);

  return (
    <div>
      {count}
      <button onClick={onClick} />
    </div>
  );
};

export default connect(ComponentName);

@SkoomaCowboy
Copy link
Author

@jimbolla that's why it has warning - proof of concept at the top :)

forceUpdate on every store state change

Nope, only on those components that are actually using the state. Similar as the current connect. Why would I use connect if I updated everything on every change? Connect memoizes a slice of the state, current mapStateToProps does that manually, I say that it should be done automatically.

The only performance hit is that every redux.state access triggers a Proxy to add it to the current listener list (if rendering, not triggered in async calls). But for all render/listen/setstate I could reuse the current connect implementation, so no performance difference in that regard.

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

2 participants