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

Allow connect() to hoist non-react statics from wrapped component #127

Merged
merged 1 commit into from
Sep 29, 2015
Merged

Allow connect() to hoist non-react statics from wrapped component #127

merged 1 commit into from
Sep 29, 2015

Conversation

erikras
Copy link
Contributor

@erikras erikras commented Sep 28, 2015

gaearon added a commit that referenced this pull request Sep 29, 2015
Allow connect() to hoist non-react statics from wrapped component
@gaearon gaearon merged commit 2f7ec22 into reduxjs:master Sep 29, 2015
@gaearon
Copy link
Contributor

gaearon commented Oct 5, 2015

Out in 3.1.0.

@erikras
Copy link
Contributor Author

erikras commented Oct 5, 2015

Hmm... The way greenkeeper references each PR with a hyperlink might get old. Do we really need to know about every public repo that merged this?

@gaearon
Copy link
Contributor

gaearon commented Oct 5, 2015

I don't really mind. The references are because it mentions release notes which link to this PR.

@erikras
Copy link
Contributor Author

erikras commented Oct 5, 2015

Yes, that's what I meant. It uses the commit text, which should reference issues and PRs it fixes/merges.

I guess as long as it doesn't generate emails. 😄

@CrocoDillon
Copy link

This is sweet, but how do you get redux' state and dispatch in the static methods?

I was hoping that since the static method is hoisted I could just get the connected component using this in the static method. However react-redux calls onEnter hooks with the route as this. I can get the constructor of the connected component (this.component) but not the instance. ¯_(ツ)_/¯

Using it like this:

// routes.jsx
<Route path="/" component={MyComponent} onEnter={MyComponent.onEnter}>

// MyComponent.jsx
class MyComponent extends Component {
  static onEnter() {
    // I need to access store.getState and store.dispatch here
  }
  render() {
    // ...
  }
}

However weird it is to access instance specific stuff in a static method. It's making server-side rendering a lot easier when you can call component methods that dispatch actions before calling ReactDOMServer.renderToString(...).

@gaearon
Copy link
Contributor

gaearon commented Jan 2, 2016

It's better to create a new issue than post in a not quite related one. Static methods don't have dispatch because it only exists in a specific rendering context. For example in a server rendered app you would render components with a different store instance on every request. This is why it doesn't make sense for static methods to have access to the dispatch function.

What I think you can do is define createRoutes(store). It would use the store to provide dispatch to the hook: onEnter={(...args) => MyComponent.onEnter(...args, store.dispatch)}.

Then you would call createRoutes(store) when rendering with your store instance. The component would receive dispatch as the last argument.

@CrocoDillon
Copy link

Sorry about that, found this PR after some searching and thought it was related enough since the hoisting is exactly what I was looking for. And yeah it did feel dirty abusing a static method like that, anyway I thought the method was hoisted to the connected component instance (and as such no longer static). I looked wrong though, it's still static so you're right... what I wanted doesn't make any sense.

The createRoutes(store) seems like the way to go! And it's really awesome that you respond so quickly :D Thanks a lot!

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

Successfully merging this pull request may close these issues.

3 participants