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

Expanded API docs for context.router #3346

Merged
merged 2 commits into from
Apr 18, 2016

Conversation

Download
Copy link
Contributor

Explain usage of context.router for:

  • Components created with React.createClass
  • Components created with ES classes extending React.Component
  • Components created as stateless functions

Explain usage of `context.router` for:
* Components created with `React.createClass`
* Components created with ES classes extending `React.Component`
* Components created as stateless functions
```js
var MyComponent = React.createClass({
contextTypes: {
router: React.PropTypes.object.isRequired
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep consistent with the other docs, can you make this router: Router.PropTypes.router?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timdorr wdyt of renaming this to routerShape so users can just do

import { routerShape } from 'react-router'

without any issues with collisions, or having to do e.g.

import { * as Router } from 'react-router'

?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd have to do that on next. But we can completely overhaul/redo the prop types at that point, so it's whatever we want it to be.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do the deprecations/&c. in place. I have them on #3349, and I think it'd be a good fix to get in place for 2.3.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep consistent with the other docs, can you make this router: Router.PropTypes.router?

Yes, will do.

* Replaced occurances of `React.PropTypes.object.isRequired` with `Router.PropTypes.router`
* Expanded abbreviation 'i.c.w' to 'in combination with'
remix-run#3346
@timdorr
Copy link
Member

timdorr commented Apr 18, 2016

Thanks for the updates!

@timdorr timdorr merged commit 9936099 into remix-run:master Apr 18, 2016
@Download
Copy link
Contributor Author

Great, thanks for merging it! 👍


```js
class MyComponent extends React.Component {
static contextTypes = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't part of ES6.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you talking about the static keyword?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah; I'll update this part, just waiting to see where we got with #3349.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's just a proposal right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I forgot about that. Babel makes using more experimental features very easy :)

@ryanflorence
Copy link
Member

ryanflorence commented Apr 18, 2016

This has me thinking we should make a withRouter higher-order component.

import { withRouter } from 'react-router'
withRouter(createClass | ES class | function)

// in the component
this.props.router

Then, it'll all be the same. This also shields apps from context changes in the future.

@taion
Copy link
Contributor

taion commented Apr 18, 2016

👍 I'm going to clean up these docs a bit, then cut 2.3.0.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants