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

Access react component context in mapStateToProps() #289

Closed
damienleroux opened this issue Feb 15, 2016 · 13 comments
Closed

Access react component context in mapStateToProps() #289

damienleroux opened this issue Feb 15, 2016 · 13 comments

Comments

@damienleroux
Copy link

Hi,

I know that binding the component context to props as been discussed and is not relevant yet.

What about reading the component own context. I need something like:
connect((appState, ownProps, ownContext) => { ... })

Is that possible?

Using react-router, I need to access some url params through inner components. I want to pass them through contextType. Then those url params are used in mapStateToProps() function to target the right stores on appState.

@gaearon
Copy link
Contributor

gaearon commented Feb 15, 2016

We don’t want to expose context in the public API because it is unstable API and will likely change. We can rely on it internally because we know our internal use case will still be supported but we don’t want to offer a public API like this and then break everyone if React does something like the “single context parent” thing they promised to do.

If you’d like to read something from context, please read it in a parent component, and pass it to the connect()ed component. Then it will be able to read that value from ownProps.

@gaearon gaearon closed this as completed Feb 15, 2016
@damienleroux
Copy link
Author

Yes It is actually the solution I implemented . Thank you for the advices.

I didn't read anything about what will become this context. Could you give me a link so I could stay up to date with React future changes?

Thank you!

@gaearon
Copy link
Contributor

gaearon commented Feb 16, 2016

I didn't read anything about what will become this context. Could you give me a link so I could stay up to date with React future changes?

There’s no specific link (it was a discussion on Twitter) but AFAIK the plan it to allow a component to receive only a single kind of context. So instead of contextTypes you’d have something like contextType. If connect() wants to receive store from context, it wouldn’t be able to read anything else from it.

@lauterry
Copy link

lauterry commented Mar 1, 2016

Hi @gaearon

Since React-Router allows you get access to the router, history, location etc.... via the context, how can you get access to the context from mapStateToProps() ?

Since its now a documented feature of React, react-redux should expose context in the public api, right ?

Best regards ?

@gaearon
Copy link
Contributor

gaearon commented Mar 1, 2016

I believe I answered this in #289 (comment). We also can’t possibly read arbitrary context because we’d need to know your contextTypes.

Please pass this info as props from parent.

@adriancooney
Copy link

You seem pretty set on your decision, Dan, but I thought I'd give you some perspective my situation where this feature might be useful for consideration.

Each of my <Route /> components are connect'd so if they're linked directly, they know what state to load. For example:

<Route path=":username/:repo" component={Repo}>
    <Route path="issues" component={Issues} />
</Route>

The Repo connected component loads the data it needs to display and selects it from the state in it's mapStateToProps. Say, for instance, it selects the repoOwner and repoInfo from the state. The child component Issues loads the data it needs to display and selects it from the state. It also needs the repoOwner and repoInfo state too so instead of performing the selection again (which is quite complicated), it's passed down via context. The problem arises when the Issues component needs information from repoOwner in it's mapStateToProps, which isn't available because it's in the context.

My hacky solution is to just create a wrapper component that passes the context as a prop but it feels dirty and I end up with a lot of useless components filling up the tree:

class IssuesWrapper extends Component {
    static contextTypes = {
        repoOwner: React.PropTypes.object
    }

    render() {
        return <Issues repoOwner={this.context.repoOwner} />
    }
}

<Route path=":username/:repo" component={Repo}>
    <Route path="issues" component={IssuesWrapper} />
</Route>

I feel as though a third argument to mapStateToProps, like React's lifecycle methods, with the context could be pretty useful so I could access repoOwner directly instead of creating the wrapper. This may be hard to implement, however, given the magic react-redux does with the function.length on the mapStateToProps function (i.e. if you want context, you have to suffer the performance impact from including props argument) so I understand it's a tough decision.

@romulof
Copy link

romulof commented Jun 28, 2016

I've made a "Provider" component to expose my Axios instance it in a context, allowing container components to inject them in my thunk dispatchers, but I think I'm doing to much repeated work, something that connect could help.

I've done similar things using props, and it looked pretty good:

// Action creator
export const fetchSorting = (sorting) => (param1, param2) => (dispatch, getState) => { ... }
// Component
class SortedList extends React.Component { ... }

const mapDispatchToProps = (dispatch, ownProps) => ({
  doFetch: bindActionCreators(fetchSorting(ownProps.sorting), dispatch),
});

export default connect(undefined, mapDispatchToProps)(SortedList);

The authors say, the usage of context is discouraged, but passing down the tree as a prop is quite painful, and I repeating the steps of react-redux and creating my own Provider/connect would be an overkill.

@mnpenner
Copy link

mnpenner commented Jan 18, 2017

Can you at least make it available to connectAdvanced? e.g.

function selectorFactory(dispatch, factoryOptions) {
    return (nextState, nextProps, nextContext) => {
        return Object.assign({}, nextProps, mapStateToProps(nextState, nextProps, nextContect), mapDispatchToProps(dispatch, nextProps, nextContect));
    };
}

@jimbolla
Copy link
Contributor

jimbolla commented Jan 18, 2017

See #599 (comment) and #599 (comment)

@mnpenner
Copy link

mnpenner commented Jan 18, 2017

@jimbolla By only injecting it into connectAdvanced, we're limiting the arity problem. You don't have to worry about how many args mapStateToProps or mapDispatchToProps take because the user is now in control of that.

Whether or not the context is passed to the inner function of selectorFactory could be a factory option; perhaps {contextTypes: xxx}, same as what the React Component and recompose accept.

reselect is a separate package; react-redux isn't responsible for that. Regardless, I think most of us are just going to copy the relevant context into the props, and then reselect will be able to handle it just fine. That's exactly what I've done, and it's working great so far.

I did manage to get getContext working though, so thank you for that! This would have been a big pain without it. I suppose I'm less concerned about this now that you've posted a reasonable workaround, but it doesn't sound like it would be that difficult to add. I think the lifecycle problem can just be ignored -- context updates simply won't be propagated since React doesn't really support it anyway.

@jimbolla
Copy link
Contributor

Until the day comes that React has a fully blessed context API, adding support for it is a maintenance liability for React Redux.

@zergione
Copy link

zergione commented Feb 10, 2017

@adriancooney what solution have you implemented in the end?

My dirty way:

const mapDispatchToProps = (dispatch) => {
    return {
        myFunction: (contextValue) => dispatch(myAction(contextValue)),
    };
};

and in the component:

const MyComponent = (props, context) => {
    return <button onClick={() => myFunction(context.value)}>go</button>
}

@adriancooney
Copy link

adriancooney commented Feb 11, 2017

@zergione in the end I opted to manage the state globally. I took a bit more effort but it felt more organised within the app and not tied to the implementation.

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

No branches or pull requests

8 participants