Use new Context API in React 16.3 #5908

timdorr · 2018-01-29T04:16:55Z

Update!

OK, a test release is on npm under the next tag.

npm install react-router@next react-router-dom@next

It's just those two packages for now, but please give them a try. Report any problems in this PR.

Continued...

Closes #5901.

~~This will obviously not work on Travis until 16.3.0 is out.~~ But I can confirm it works locally.

To play along at home, simply clone react in a parallel folder, yarn && yarn run build, and then copy over the files by hand back to this repo:

cp -a ../react/build/node_modules/react/* node_modules/react/
cp -a ../react/build/node_modules/react-dom/* node_modules/react-dom/

This is mostly a brute force approach. All the tests pass, but I'm doing whatever is necessary to get it to work. There may be some more radical refactors warranted with the new API. The new bitmask stuff is neato.

While not BC yet, I'm least keeping available the old context API hooks. I need to feature sniff unstable_createContext.

Outstanding stuff:

timdorr · 2018-01-29T04:18:49Z

packages/react-router/modules/RouterContext.js

@@ -0,0 +1,5 @@
+import React from "react";
+
+const RouterContext = React.unstable_createContext(0);


Not sure if {} is a valid default value, but it would be better.

I believe it should be the shape of the expected context. Something like this, but I'm not sure how specific it is supposed to be.

React.unstable_createContext({ history: null, route: { location: null, match: null } });

The <StaticRouter> also places a staticContext on the context, but whether it is necessary to include it I cannot say.

It can be whatever you want it to be. null seems like an appropriate choice.

pshrmn · 2018-01-29T06:29:57Z

packages/react-router/modules/Router.js

@@ -77,7 +78,10 @@ class Router extends React.Component {

  render() {
    const { children } = this.props;
-    return children ? React.Children.only(children) : null;
+    return RouterContext.provide(
+      this.getChildContext(),


Maybe this should be this.getChildContext().router and then it can be consumed directly.

const Cmp = props => ( RouterContext.consume(router => <InnerCmp {...props} router={router} />) );

Tried that. Because of how the initial render works under this API, I ran into a boatload of errors.

timdorr · 2018-01-29T18:47:14Z

Looks like the RRN stuff is going to have to stick with the old context API (still supported!) for now. Changing it out is fairly easy, but our tests need more extensive changes to support rendering beyond the consumer. And I'm not 100% sure how to do that. Maybe use enzyme? But that's an equally large ask.

Anyways, punting on RRN for the moment.

pshrmn · 2018-01-29T19:05:05Z

Enzyme won't work because it needs to do full mounts to render both the provider and consumer. However, from what I can tell, React Native components don't work with Enzyme's mount.

Even with regular React there are Enzyme issues because you can only update the props for the root component. An extra wrapper can probably be added to get around this, but I haven't played around with that yet.

These issues are more "things that don't work now" than "things that wont' work", because there is no reason to expect Enzyme to work with an unstable API, but I figured I'd throw them out there.

timdorr · 2018-01-29T19:20:33Z

Who would have thought an experimental feature that landed in an upstream library's master 5 days ago would have trouble being supported by other libraries? /s

pshrmn · 2018-01-29T20:27:46Z

*opens window* Its new context API and I need it NOW!

Having used the new context for a bit, did you have any thoughts on how to maintain old context support?

timdorr · 2018-01-29T20:47:26Z

Just maintain getChildContext() and use it as your input value to .produce(). That way is easy. It's harder to go back to <= 16.2 support. I don't have an answer for that yet, other than basically ponyfilling the API.

pshrmn · 2018-01-29T21:23:56Z

From a maintenance perspective, a clean break is the nicest solution. I have been toying with react-broadcast in my router so that I can mimic (sort of, the API isn't exactly the same) the new context and besides the testing issues everything works well. It might make sense to make that same switch here and transition to the real context whenever that becomes official.

pshrmn · 2018-01-30T04:41:07Z

I did some digging into react-test-renderer. I believe that the RN tests should look something like this:

import renderer from 'react-test-renderer';
import { createMemoryHistory } from 'history';

const createHistory = () => {
  const history = createMemoryHistory();
  history.push = jest.fn();
  history.replace = jest.fn();
  return history;
};

describe('...', () => {
  it('...', () => {
    const history = createHistory();
    const output = renderer.create((
      <Router history={history}>
        <Link to='/push' />
      </Router>
    ));

    const event = new EventStub()
    // find the link and "press" it
    const link = output.root.findByType(Link);
    link.props.onPress(event)

    expect(history.push.mock.calls.length).toBe(1)
    expect(history.push.mock.calls[0][0]).toBe('/push')
  });
});

pshrmn · 2018-02-03T01:44:18Z

16.3.0-alpha.0 was released with Context.provide/Context.consume replaced by <Context.Provider> and <Context.Consumer>.

timdorr · 2018-02-03T02:37:33Z

Welp, that's a little bit of work to implement. But now I can get Travis to pass 👍

pshrmn · 2018-02-03T02:54:53Z

Yeah, the lack of an href-like attribute on React Native links makes testing where they will navigate to a bit convoluted. The above example could probably be a bit shorter because the history instance only really needs to be mocked if you're verifying the navigation happens.

I'm excited for 16.3. They also dropped by unstable_ prefix on createContext, so hopefully that is a sign of confidence.

timdorr · 2018-02-06T15:49:54Z

OK, I added in create-react-context to polyfill for React <= 16.2. I used that to downgrade back to the current stable release, which ensures RRN is in sync.

Lerna's hoisting, combined with the symlinking between packages, was causing two versions of React to sneak into the tests. I'm not sure how best to fix that, but the easiest thing is to run against the stable version. It ensures we're backwards compatible as well.

I think we're good to go now. Just need a review and some feedback on how this turned out.

timdorr · 2018-02-09T21:00:27Z

Just so others are aware of the plan/goal here: I'm holding off on merging this into master until we get our next release out. There are a lot of useful things queued up and this change is large enough that I don't want to poison the well on what should be a high quality release.

Afterwards, I'll look to getting this merged and released as a prerelease to allow for everyone to test it and give feedback. I'm most nervous about the component tree changes (a bunch more of stuff will show up with this in place!) and unintended consequences too.

pravdomil · 2018-02-22T20:46:28Z

@timdorr can we try it now? Is there any prerelease?

timdorr · 2018-02-22T21:09:59Z

You can check out this PR and build it locally.

k15a · 2018-04-30T14:29:56Z

packages/react-router/modules/RouterContext.js

+import React from "react";
+import createReactContext from "create-react-context";
+
+const RouterContext = React.createContext


That's already done in create-react-context https://github.com/jamiebuilds/create-react-context/blob/master/src/index.js

Yep, this changed since the last time I touched this PR. Since I have to resolve a bunch of package-lock.json file conflicts anyways, I'll get this swapped out at that point.

techniq · 2018-05-31T14:57:16Z

@timdorr any outstanding issues left for this PR?

jacargentina · 2018-07-04T21:09:03Z

@timdorr I'm trying to test this with no luck, the API changes in some way? There is some docs on the needed changes? withRouter still works? RouterContext/Provider should i put somwhere? Thanks!

timdorr · 2018-07-04T21:19:43Z

No API changes needed. The only thing is a new RouterContext export, but it should be fully BC, even with legacy Context.

bdwain · 2018-08-03T15:37:55Z

@timdorr do you have any info on how long you want to keep this in alpha before getting ready to release it?

DeanBDean · 2018-08-13T20:37:49Z

Hey @timdorr, is there any way you can push an update for an alpha branch of the react-router-config to include the alpha branch of react-router? I was trying to test these changes, but unfortunately we use react-router-config and I believe it's using the non alpha version of react-router.

EDIT:
Turns out the issue I was seeing was deeper than just the version of react-router in react-router-config. renderRoutes references Switch and Route from react-router. Unfortunately, if we use renderRoutes on the clientSide, we start getting errors that say You should not use <Switch> outside a <Router>. The source of these errors is that BrowserRouter from react-router-dom and Switch/Route from react-router have different RouterContexts. I was able to work around this by creating a version of of renderRoutes that referenced Switch and Route from react-router-dom.

This seems like a bit of a sticky issue. I am not sure if you are partial to any one way of tackling this. If you have something in mind, I don't mind tackling it in a branch of this one to merge into this PR.

mjackson

Looks pretty good, @timdorr 😅 Thanks for taking this on!

mjackson · 2018-09-20T17:37:25Z

packages/react-router-dom/modules/index.js

@@ -7,6 +7,7 @@ export Prompt from "./Prompt";
 export Redirect from "./Redirect";
 export Route from "./Route";
 export Router from "./Router";
+export RouterContext from "./RouterContext";


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.

@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.

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

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

@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?

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

@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.

mjackson · 2018-09-20T17:39:09Z

packages/react-router/modules/Router.js

@@ -23,7 +21,7 @@ class Router extends React.Component {
  getChildContext() {
    return {
      router: {


No need to use the router key here. Let's just put everything directly on the context object.

This is to maintain BC with those that are relying on our existing legacy Context usage.

mjackson · 2018-09-20T17:40:11Z

packages/react-router/modules/__tests__/Router-test.js

@@ -50,17 +51,14 @@ describe("A <Router>", () => {

  describe("context", () => {


Let's go ahead and remove all tests that test our context values in this PR. Since context is not part of our public API, we shouldn't have them.

mjackson · 2018-09-20T17:42:23Z

packages/react-router/modules/index.js

@@ -3,6 +3,7 @@ export Prompt from "./Prompt";
 export Redirect from "./Redirect";
 export Route from "./Route";
 export Router from "./Router";
+export RouterContext from "./RouterContext";


Again, let's not expose our context API as public API.

We need to export it to get it between the react-router and react-router-dom modules.

mjackson · 2018-09-20T17:42:49Z

website/modules/ReactRouterDOMShim.js

@@ -9,6 +9,7 @@ export {
  Redirect,
  Route,
  Router,
+  RouterContext,


Should be able to remove this as well.

0.4.16 has a bug. See oliviertassinari/babel-plugin-transform-react-remove-prop-types#160

mjackson · 2018-09-20T19:25:20Z

Thanks, @timdorr! I'm going to make a few tweaks and go ahead and cut a 4.4 beta release with these changes.

timdorr commented Jan 29, 2018

View reviewed changes

pshrmn reviewed Jan 29, 2018

View reviewed changes

pshrmn mentioned this pull request Jan 29, 2018

Use new Context API in React 16.3 #5901

Closed

timdorr changed the title ~~[WIP] Use new Context API in React 16.3~~ Use new Context API in React 16.3 Feb 6, 2018

ricardobrandao approved these changes Feb 9, 2018

View reviewed changes

timdorr mentioned this pull request Mar 10, 2018

React 16 experiment: rewrite React-Redux to use new context API reduxjs/react-redux#898

Closed

jgbright mentioned this pull request Apr 15, 2018

Error while using with React-Router didierfranc/react-waterfall#35

Closed

remix-run deleted a comment from hlehmann Apr 21, 2018

timdorr force-pushed the master branch 4 times, most recently from 8f33936 to 1fb8696 Compare April 26, 2018 20:30

k15a reviewed Apr 30, 2018

View reviewed changes

techniq mentioned this pull request May 31, 2018

Deprecate/remove withRouter? #5345

Closed

timdorr force-pushed the react-context-api branch from 6f7e912 to 35c35af Compare June 6, 2018 17:04

mjackson reviewed Sep 20, 2018

View reviewed changes

timdorr and others added 16 commits September 20, 2018 14:22

Use the new 16.3.0 context API.

efc7c43

Export so others can use it.

19cb4d9

Only children again.

b7c3efa

Better default value

1675aa4

Make Prompt use the new API

58a1ee3

Make Redirect use the new context API

84c0d03

Export RouterContext in rrd

1e6116e

Use RouterContext in Link

c8c9429

Make StaticRouter inheritence work

2a5d221

Directly consume context in Link.

15e39d8

Update to final component-based context API

f0328c3

Use create-react-context for BC

dcfc79e

Revert back to 16.2 to check BC and sync up with RRN.

0136d3d

Update create-react-context and let it pass through.

4db0bfe

Fix packaging/build issues.

971aec3

im -> ex

6124f69

timdorr force-pushed the react-context-api branch from 8498f91 to 6124f69 Compare September 20, 2018 18:24

Downgrade babel-plugin-transform-react-remove-prop-types

63cbf11

0.4.16 has a bug. See oliviertassinari/babel-plugin-transform-react-remove-prop-types#160

mjackson merged commit 449a528 into master Sep 20, 2018

mjackson deleted the react-context-api branch September 20, 2018 19:25

frehner mentioned this pull request Sep 20, 2018

Deprecated lifecycles #6341

Closed

quantizor mentioned this pull request Sep 23, 2018

v4 beta constructive feedback styled-components/styled-components#1969

Closed

stevenmusumeche mentioned this pull request Oct 4, 2018

Weird problem when using StyleRoot component with react-router FormidableLabs/radium#1000

Closed

lock bot locked as resolved and limited conversation to collaborators Nov 24, 2018

		@@ -0,0 +1,5 @@
		import React from "react";

		const RouterContext = React.unstable_createContext(0);

		@@ -50,17 +51,14 @@ describe("A <Router>", () => {

		describe("context", () => {

Use new Context API in React 16.3 #5908

Use new Context API in React 16.3 #5908

Conversation

timdorr commented Jan 29, 2018 • edited

Update!

Continued...

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timdorr commented Jan 29, 2018 • edited

pshrmn commented Jan 29, 2018

timdorr commented Jan 29, 2018

pshrmn commented Jan 29, 2018

timdorr commented Jan 29, 2018

pshrmn commented Jan 29, 2018

pshrmn commented Jan 30, 2018

pshrmn commented Feb 3, 2018

timdorr commented Feb 3, 2018

pshrmn commented Feb 3, 2018

timdorr commented Feb 6, 2018

timdorr commented Feb 9, 2018

pravdomil commented Feb 22, 2018 • edited

timdorr commented Feb 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techniq commented May 31, 2018

jacargentina commented Jul 4, 2018

timdorr commented Jul 4, 2018

bdwain commented Aug 3, 2018

DeanBDean commented Aug 13, 2018 • edited

mjackson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjackson commented Sep 20, 2018

timdorr commented Jan 29, 2018 •

edited

timdorr commented Jan 29, 2018 •

edited

pravdomil commented Feb 22, 2018 •

edited

DeanBDean commented Aug 13, 2018 •

edited