Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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
v12 merged 2 commits into react-bootstrap:master from kfitzgerald:context-fix
Mar 19, 2019

Conversation

Copy link
Contributor

@kfitzgerald kfitzgerald commented Mar 18, 2019

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

swarthy, taijuten, ericgio, travisdieckmann, gurkerl83, Englund0110, mfakhrusy, and meetajhu reacted with thumbs up emoji swarthy reacted with hooray emoji
 * 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
Copy link
Member

v12 commented Mar 19, 2019

This is great, thanks!

kfitzgerald reacted with thumbs up emoji

Copy link

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

AlmostBearded, jayair, kris2kris, davidlewallen, meetajhu, RopoMen, lvpro, and SaymV reacted with eyes emoji

Copy link

lvpro commented Mar 26, 2019
edited
Loading

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.

Copy link

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

Copy link

A workaround is available here #250

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

Copy link
Member

taion commented Mar 27, 2019

released in v0.25.0

kfitzgerald, kris2kris, tmcknight, thetrevorharmon, ArnoSaine, mxschmitt, and swarthy reacted with hooray emoji

Copy link

@taion thank you!

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!

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
Reviewers

@v12 v12 Awaiting requested review from v12

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /