Skip to content

Use new Context API in React 16.3 #5908

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.

Sign up for GitHub

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 17 commits into from
Sep 20, 2018
Merged

Use new Context API in React 16.3 #5908

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
efc7c43
Use the new 16.3.0 context API.
timdorr Jan 29, 2018
19cb4d9
Export so others can use it.
timdorr Jan 29, 2018
b7c3efa
Only children again.
timdorr Jan 29, 2018
1675aa4
Better default value
timdorr Jan 29, 2018
58a1ee3
Make Prompt use the new API
timdorr Jan 29, 2018
84c0d03
Make Redirect use the new context API
timdorr Jan 29, 2018
1e6116e
Export RouterContext in rrd
timdorr Jan 29, 2018
c8c9429
Use RouterContext in Link
timdorr Jan 29, 2018
2a5d221
Make StaticRouter inheritence work
timdorr Jan 29, 2018
15e39d8
Directly consume context in Link.
timdorr Jan 29, 2018
f0328c3
Update to final component-based context API
timdorr Feb 3, 2018
dcfc79e
Use create-react-context for BC
timdorr Feb 6, 2018
0136d3d
Revert back to 16.2 to check BC and sync up with RRN.
timdorr Feb 6, 2018
4db0bfe
Update create-react-context and let it pass through.
timdorr Jun 6, 2018
971aec3
Fix packaging/build issues.
timdorr Jun 6, 2018
6124f69
im -> ex
timdorr Jun 26, 2018
63cbf11
Downgrade babel-plugin-transform-react-remove-prop-types
timdorr Sep 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
308 changes: 159 additions & 149 deletions packages/react-router-config/package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/react-router-config/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"babel-jest": "^23.0.1",
"babel-plugin-dev-expression": "^0.2.1",
"babel-plugin-external-helpers": "^6.22.0",
"babel-plugin-transform-react-remove-prop-types": "^0.4.13",
"babel-plugin-transform-react-remove-prop-types": "0.4.15",
"babel-preset-es2015": "^6.14.0",
"babel-preset-react": "^6.5.0",
"babel-preset-stage-1": "^6.5.0",
Expand Down
48 changes: 23 additions & 25 deletions packages/react-router-dom/modules/Link.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from "react";
import PropTypes from "prop-types";
import invariant from "invariant";
import { createLocation } from "history";
import RouterContext from "./RouterContext";

const isModifiedEvent = event =>
!!(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey);
Expand All @@ -22,17 +23,7 @@ class Link extends React.Component {
replace: false
};

static contextTypes = {
router: PropTypes.shape({
history: PropTypes.shape({
push: PropTypes.func.isRequired,
replace: PropTypes.func.isRequired,
createHref: PropTypes.func.isRequired
}).isRequired
}).isRequired
};

handleClick = event => {
handleClick = history => event => {
if (this.props.onClick) this.props.onClick(event);

if (
Expand All @@ -43,7 +34,6 @@ class Link extends React.Component {
) {
event.preventDefault();

const { history } = this.context.router;
const { replace, to } = this.props;

if (replace) {
Expand All @@ -57,22 +47,30 @@ class Link extends React.Component {
render() {
const { replace, to, innerRef, ...props } = this.props; // eslint-disable-line no-unused-vars

invariant(
this.context.router,
"You should not use <Link> outside a <Router>"
);

invariant(to !== undefined, 'You must specify the "to" property');

const { history } = this.context.router;
const location =
typeof to === "string"
? createLocation(to, null, null, history.location)
: to;

const href = history.createHref(location);
return (
<a {...props} onClick={this.handleClick} href={href} ref={innerRef} />
<RouterContext.Consumer>
{({ router }) => {
invariant(router, "You should not use <Link> outside a <Router>");

const { history } = router;
const location =
typeof to === "string"
? createLocation(to, null, null, history.location)
: to;

const href = history.createHref(location);
return (
<a
{...props}
onClick={this.handleClick(history)}
href={href}
ref={innerRef}
/>
);
}}
</RouterContext.Consumer>
);
}
}
Expand Down
3 changes: 3 additions & 0 deletions packages/react-router-dom/modules/RouterContext.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Written in this round about way for babel-transform-imports
import { RouterContext } from "react-router";
export default RouterContext;
5 changes: 1 addition & 4 deletions packages/react-router-dom/modules/__tests__/Link-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,7 @@ describe("A <Link>", () => {
ReactDOM.render(<Link to="/">link</Link>, node);
}).toThrow(/You should not use <Link> outside a <Router>/);

expect(console.error.calls.count()).toBe(3);
expect(console.error.calls.argsFor(0)[0]).toContain(
"The context `router` is marked as required in `Link`"
);
expect(console.error.calls.count()).toBe(2);
});

it('throws with no "to" prop', () => {
Expand Down
1 change: 1 addition & 0 deletions packages/react-router-dom/modules/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export Prompt from "./Prompt";
export Redirect from "./Redirect";
export Route from "./Route";
export Router from "./Router";
export RouterContext from "./RouterContext";
Copy link
Member

Choose a reason for hiding this comment

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

Let's not export RouterContext as part of our public API. It's an implementation detail that users shouldn't care about. If they want routing data, they can always render a <Route> or use withRouter.

Copy link

Choose a reason for hiding this comment

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

@mjackson, what do you think about exporting the context's <Consumer /> as a render-prop version of withRouter HOC? withRouter brings addition effort since the piece that needs the data from the router needs to be moved to a separate component that uses the HOC.

Copy link
Member Author

Choose a reason for hiding this comment

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

@alexeyraspopov <Route> has a render prop API, both with the render prop and a children-as-a-function "prop".

Copy link

Choose a reason for hiding this comment

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

Oh, I've just found out that path is not required. Worth trying, thank you.

Copy link

Choose a reason for hiding this comment

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

@mjackson what about a case where an application is using a Router nested inside another Router and the nested Router contains a NavLink that would like to use the parent Router's Context... if the Context is exported it can be explicitly used in this situation. Thoughts?

Copy link

Choose a reason for hiding this comment

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

@crobinson42 Is such a thing possible today with the old context system? If not, isn't that out of scope for this PR?

Copy link

Choose a reason for hiding this comment

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

@josephcsible I have not been able to find a solution using the old context system. I suppose it would be considered in-scope since this PR would provide the opportunity to use the library in a new way by the benefits of the new context api. But, I do see your point in the requirement it would add to then add a prop to the wrapping <Router context={SpecificRouterContext}> component.

export StaticRouter from "./StaticRouter";
export Switch from "./Switch";
export generatePath from "./generatePath";
Expand Down
Loading