Skip to content

Use Public API for React-Router instead of Private Context #248

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

Merged
merged 2 commits into from
Mar 19, 2019

Conversation

kfitzgerald
Copy link
Contributor

Per: https://github.com/ReactTraining/react-router/releases/tag/v4.4.0-beta.0

We also made it explicit in this release that you can't use contextTypes to access properties on this.context.router anymore. If you try, you'll get a warning. That's our private API! If you need stuff on context, just use a or withRouter instead. It's all the same stuff, and that's our public API 👍

🚨 In a future release, we will remove the old context API entirely so your app will break if you were using our private API. 🚨

Since react-router-bootstrap uses the forbidden contextTypes, this PR addresses that.

In short, LinkContainer is now exported by default, wrapped by withRouter. This exposes history and such directly as a property on LinkContainer.

Additionally, the original unwrapped LinkContainer class is exported, if desired.

Summary of changes:

  • IndexLinkContainer:
    • Wrapped withRouter as default export,
    • Now exports original component separately
  • LinkContainer:
    • Wrapped withRouter as default export,
    • Now exports original component separately
    • contextTypes validation has been removed
    • propTypes now expects history object (required), as it is expected to be wrapped withRouter
    • Added propTypes: location, match, staticContext, since withRouter will provide them
    • this.context.router.history references replaced with this.props.history
    • Updated render() to strip props (location, match, staticContext) from children
  • Tests updated to deal with wrapped component vs real component references
  • git ignore updates

Fixes #237

 * IndexLinkContainer:
   * Wrapped `withRouter` as default export,
   * Now exports original component separately
 * LinkContainer:
   * Wrapped `withRouter` as default export,
   * Now exports original component separately
   * `contextTypes` validation has been removed
   * `propTypes` now expects `history` object, as it is expected to be wrapped `withRouter`
   * `this.context.router.history` references replaced with `this.props.history`
   * Tests updated to deal with wrapped component vs real component references
 * git ignore updates
 * Made `history` propType required
 * Added propTypes: `location`, `match`, `staticContext`, since `withRouter` will provide them
 * Updated `render()` to strip props (`location`, `match`, `staticContext`) from children
@taion taion requested a review from v12 March 18, 2019 12:53
@v12 v12 merged commit 27b1e9c into react-bootstrap:master Mar 19, 2019
@v12
Copy link
Member

v12 commented Mar 19, 2019

This is great, thanks!

@Englund0110
Copy link

For curiosity, when can we expect a new release with this change?

@lvpro
Copy link

lvpro commented Mar 26, 2019

For curiosity, when can we expect a new release with this change?

I agree. LinkContainer is broken for anyone upgrading to react-router 5.x, so breadcrumbs stop working without hacky solutions that lead to nested anchors and validateDOMnesting warnings.

@thetrevorharmon
Copy link

+1 to a release. Also feeling the pain as I'm working with react-router v5. @v12 any timeline?

@kris2kris
Copy link

A workaround is available here #250

@taion
Copy link
Member

taion commented Mar 27, 2019

@v12 are you v12 on npm? terribly sorry i forgot to add you as a publisher. i'll publish this one though

@taion
Copy link
Member

taion commented Mar 27, 2019

released in v0.25.0

@thetrevorharmon
Copy link

@taion thank you!

@v12
Copy link
Member

v12 commented Apr 5, 2019

@taion I am, yes but I do use v6 to publish packages. I'd appreciate if you add either of these accounts as a publisher. Thanks!

@taion
Copy link
Member

taion commented Apr 8, 2019

@v12 done

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.

7 participants