Use constructor instead of cWM in <Redirect>

remix-run · Sep 25, 2018 · 427db56 · 427db56
1 parent 33c6f6f
commit 427db56
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 76 deletions.
diff --git a/packages/react-router/modules/Redirect.js b/packages/react-router/modules/Redirect.js
@@ -23,29 +23,22 @@ function computeTo(props) {
 }
 
 /**
- * The public API for updating the location programmatically
- * with a component.
+ * The public API for updating the location programmatically with a component.
  */
 class InnerRedirect extends React.Component {
   static defaultProps = {
     push: false
   };
 
-  isStatic() {
-    return this.props.router && this.props.router.staticContext;
-  }
+  constructor(props) {
+    super(props);
 
-  componentWillMount() {
     invariant(
       this.props.router,
       "You should not use <Redirect> outside a <Router>"
     );
 
-    if (this.isStatic()) this.perform();
-  }
-
-  componentDidMount() {
-    if (!this.isStatic()) this.perform();
+    this.perform();
   }
 
   componentDidUpdate(prevProps) {

diff --git a/packages/react-router/modules/Route.js b/packages/react-router/modules/Route.js
@@ -46,12 +46,37 @@ class Route extends React.Component {
   };
 
   getChildContext() {
-    // TODO: Warn about accessing context directly. It will be removed.
+    // TODO: Warn about accessing this.context.router directly. It will be removed.
     return {
       router: getContext(this.props, this.context.router)
     };
   }
 
+  constructor(props) {
+    super(props);
+
+    if (__DEV__) {
+      warning(
+        !(props.component && props.render),
+        "You should not use <Route component> and <Route render> in the same route; <Route render> will be ignored"
+      );
+
+      warning(
+        !(
+          props.component &&
+          props.children &&
+          !isEmptyChildren(props.children)
+        ),
+        "You should not use <Route component> and <Route children> in the same route; <Route children> will be ignored"
+      );
+
+      warning(
+        !(props.render && props.children && !isEmptyChildren(props.children)),
+        "You should not use <Route render> and <Route children> in the same route; <Route children> will be ignored"
+      );
+    }
+  }
+
   render() {
     return (
       <RouterContext.Consumer>
@@ -107,31 +132,6 @@ if (__DEV__) {
     location: PropTypes.object
   };
 
-  Route.prototype.componentDidMount = function() {
-    warning(
-      !(this.props.component && this.props.render),
-      "You should not use <Route component> and <Route render> in the same route; <Route render> will be ignored"
-    );
-
-    warning(
-      !(
-        this.props.component &&
-        this.props.children &&
-        !isEmptyChildren(this.props.children)
-      ),
-      "You should not use <Route component> and <Route children> in the same route; <Route children> will be ignored"
-    );
-
-    warning(
-      !(
-        this.props.render &&
-        this.props.children &&
-        !isEmptyChildren(this.props.children)
-      ),
-      "You should not use <Route render> and <Route children> in the same route; <Route children> will be ignored"
-    );
-  };
-
   Route.prototype.componentDidUpdate = function(prevProps) {
     warning(
       !(this.props.location && !prevProps.location),

diff --git a/packages/react-router/modules/Router.js b/packages/react-router/modules/Router.js
@@ -28,21 +28,20 @@ class Router extends React.Component {
   };
 
   getChildContext() {
-    // TODO: Warn about accessing context directly. It will be removed.
+    // TODO: Warn about accessing this.context.router directly. It will be removed.
     return {
       router: getContext(this.props, this.state)
     };
   }
 
-  state = {
-    location: this.props.history.location
-  };
+  constructor(props) {
+    super(props);
+
+    this.state = {
+      location: props.history.location
+    };
 
-  componentWillMount() {
-    // Do this here so we can setState when a <Redirect> changes the
-    // location in componentWillMount. This happens e.g. when doing
-    // server rendering using a <StaticRouter>.
-    this.unlisten = this.props.history.listen(location => {
+    this.unlisten = props.history.listen(location => {
       this.setState({ location });
     });
   }
@@ -55,9 +54,10 @@ class Router extends React.Component {
     const context = getContext(this.props, this.state);
 
     return (
-      <RouterContext.Provider value={context}>
-        {this.props.children || null}
-      </RouterContext.Provider>
+      <RouterContext.Provider
+        children={this.props.children || null}
+        value={context}
+      />
     );
   }
 }

diff --git a/packages/react-router/modules/__tests__/Switch-test.js b/packages/react-router/modules/__tests__/Switch-test.js
@@ -14,7 +14,7 @@ describe("A <Switch>", () => {
   });
 
   describe("without a <Router>", () => {
-    it("throws", () => {
+    it("throws an error", () => {
       spyOn(console, "error");
 
       expect(() => {
@@ -34,80 +34,90 @@ describe("A <Switch>", () => {
       node
     );
 
-    expect(node.innerHTML).toMatch(/one/);
+    expect(node.innerHTML).toContain("one");
   });
 
-  it("renders the first <Redirect from> that matches the URL", () => {
+  it("does not render a second <Route> that also matches the URL", () => {
+    ReactDOM.render(
+      <MemoryRouter initialEntries={["/one"]}>
+        <Switch>
+          <Route path="/one" render={() => <h1>one</h1>} />
+          <Route path="/one" render={() => <h1>two</h1>} />
+        </Switch>
+      </MemoryRouter>,
+      node
+    );
+
+    expect(node.innerHTML).not.toContain("two");
+  });
+
+  it("renders the first <Redirect> that matches the URL", () => {
     ReactDOM.render(
       <MemoryRouter initialEntries={["/three"]}>
         <Switch>
           <Route path="/one" render={() => <h1>one</h1>} />
-          <Redirect from="/four" to="/one" />
-          <Redirect from="/three" to="/two" />
           <Route path="/two" render={() => <h1>two</h1>} />
+          <Redirect from="/three" to="/two" />
         </Switch>
       </MemoryRouter>,
       node
     );
 
-    expect(node.innerHTML).toMatch(/two/);
+    expect(node.innerHTML).toContain("two");
   });
 
-  it("does not render a second <Route> or <Redirect> that also matches the URL", () => {
+  it("does not render a second <Redirect> that also matches the URL", () => {
     ReactDOM.render(
-      <MemoryRouter initialEntries={["/one"]}>
+      <MemoryRouter initialEntries={["/three"]}>
         <Switch>
           <Route path="/one" render={() => <h1>one</h1>} />
-          <Redirect from="/one" to="/two" />
-          <Route path="/one" render={() => <h1>two</h1>} />
           <Route path="/two" render={() => <h1>two</h1>} />
+          <Redirect from="/three" to="/two" />
+          <Redirect from="/three" to="/one" />
         </Switch>
       </MemoryRouter>,
       node
     );
 
-    expect(node.innerHTML).not.toMatch(/two/);
+    expect(node.innerHTML).toContain("two");
   });
 
-  it("renders pathless Routes", () => {
+  it("renders a Route with no `path` prop", () => {
     ReactDOM.render(
-      <MemoryRouter initialEntries={["/cupcakes"]}>
+      <MemoryRouter initialEntries={["/two"]}>
         <Switch>
-          <Route path="/bubblegum" render={() => <div>one</div>} />
-          <Route render={() => <div>two</div>} />
+          <Route path="/one" render={() => <h1>one</h1>} />
+          <Route render={() => <h1>two</h1>} />
         </Switch>
       </MemoryRouter>,
       node
     );
 
-    expect(node.innerHTML).not.toContain("one");
     expect(node.innerHTML).toContain("two");
   });
 
-  it("handles from-less Redirects", () => {
+  it("renders a Redirect with no `from` prop", () => {
     ReactDOM.render(
-      <MemoryRouter initialEntries={["/cupcakes"]}>
+      <MemoryRouter initialEntries={["/three"]}>
         <Switch>
-          <Route path="/bubblegum" render={() => <div>bub</div>} />
-          <Redirect to="/bubblegum" />
-          <Route path="/cupcakes" render={() => <div>cup</div>} />
+          <Route path="/one" render={() => <h1>one</h1>} />
+          <Redirect to="/one" />
+          <Route path="/two" render={() => <h1>two</h1>} />
         </Switch>
       </MemoryRouter>,
       node
     );
 
-    expect(node.innerHTML).not.toContain("cup");
-    expect(node.innerHTML).toContain("bub");
+    expect(node.innerHTML).toContain("one");
   });
 
   it("handles subsequent redirects", () => {
     ReactDOM.render(
       <MemoryRouter initialEntries={["/one"]}>
         <Switch>
-          <Redirect exact from="/one" to="/two" />
-          <Redirect exact from="/two" to="/three" />
-
-          <Route path="/three" render={() => <div>three</div>} />
+          <Redirect from="/one" to="/two" />
+          <Redirect from="/two" to="/three" />
+          <Route path="/three" render={() => <h1>three</h1>} />
         </Switch>
       </MemoryRouter>,
       node