reduxjs / react-redux

Official React bindings for Redux
https://react-redux.js.org
MIT License
23.37k stars 3.37k forks source link

Proposal to render connected component only when their props are guaranteed (a la Relay) #414

Closed PCreations closed 8 years ago

PCreations commented 8 years ago

I'm using react-redux for a year now and I always find myself struggling with the same situation :

Example : Todo.js

const Todo = ({ text }) => <li>{text}</li>

TodoList.js

class TodoList extends React.Component {

    componentWillMount() {
        if (this.props.areAllTodosFetched === false) {
            this.props.fetchTodos()
        }
    }

    render() {
        return this.props.areAllTodosFetched ? (
            this.props.todos.map(t => <Todo key={t.id} text={t.text}/>)
        ) : (
            <Spinner/>
        )
    }
}

TodoList.propTypes = {
    areAllTodosFetched: React.PropTypes.bool,
    todos: React.PropTypes.arrayOf(React.PropTypes.object),
    fetchTodos: React.PropTypes.func
}

export default connect(
    (state) => ({
        areAllTodosFetched: areAllTodosFetched(state),
        todos: getTodos(state),
    }),
    (dispatch) => ({
        fetchTodos: () => dispatch(fetchTodos())
    })
)(TodoList)

I don't want the TodoList component to be responsible for fetching data, but I do want to mark it as dependent of the todos.

I'm a huge fan of the separation of concerns in redux :

...but who is responsible to know if a corresponding slice of state is not "valid" and that an action need to be triggered to make it "valid" ? We actually do this in components themselves as in my example above, but it's get tedious as the app grow because many components can have the same requirements. I would love to hear about other experiencing the same issues and how you solve them. I've made a little draft repo to expose my proposal, it's absolutely not optimal, and i'd love to have feedback before digging further, does this sound like a good idea or are there some anti-pattern ? Here is my proposal :

(I don't want to be pushy about my own work, I just want feedback because I think it's very common source of reflexion and problem)

Introducing the notion of 'pledge'

You already know what a reducer, an action creator or a selector is. React-redux-pledge introduces the notion of pledge (a better word would be a Promise but, for obvious reason, I chosen "pledge" ;) ). A pledge is an object that know if a slice of your state is valid and if not, how to make it valid

/* A typical pledge object */
const pledge = {
    isResolved(state) { ... },
    getAction() { ... }
}

isResolved receive the global state as an argument, you can select a slice of the state and return true if this slice need to be considered valid of false otherwise. getAction should returns the action that need to be dispatched in order to make the slice of the state handled by this pledge valid (typically an action creator initiating an api request).

Enhance your connected component with the withPledges higher-order component

Todo.js

const Todo = ({ text }) => <li>{text}</li>

todos.js (let's say that's a module with some action creators, reducers, etc., handling the todos)

import { createPledge } from 'react-redux-pledge'

[...]  // reducers, actions creators, etc.

const pledgeOnAllTodosFetched = createPledge(
    (state) => state.areAllTodosFetched,
    () => fetchAll()
)

TodoList.js

import { withPledges } from 'react-redux-pledge'
import { pledgeOnAllTodosFetched } from './todos'
import Spinner from './someSpinnerComponent'

const TodoList = ({ todos }) => todos.map(t => <Todo key={t.id} text={t.text}/>)

TodoList.propTypes = {
    todos: React.PropTypes.arrayOf(React.PropTypes.object),
}

const enhancer = withPledges([[pledgeOnAllTodosFetched]], Spinner)  // notice the array of array here, more on this [here](https://github.com/PCreations/react-redux-pledge)

export default connect(
    (state) => ({
        todos: getTodos(state),
    }),
)(enhance(TodoList))

It's NOT a proposal to declarative data fetching, just an helper to don't need to worry about what component is fetching what data, and just let the requirements bubble up.

gaearon commented 8 years ago

I think this could work great as a third party library. If it proves popular, we might consider including something like this by default.