Make createRender more sophisticated

examples/basic-jsx/src/index.js

@@ -88,7 +88,7 @@ const BrowserRouter = createBrowserRouter({
     </Route>
   ),
 
-  renderError: error => (
+  renderError: ({ error }) => ( // eslint-disable-line react/prop-types
     <div>
       {error.status === 404 ? 'Not found' : 'Error'}
     </div>

examples/basic/src/index.js

@@ -87,7 +87,7 @@ const BrowserRouter = createBrowserRouter({
     },
   ],
 
-  renderError: error => (
+  renderError: ({ error }) => ( // eslint-disable-line react/prop-types
     <div>
       {error.status === 404 ? 'Not found' : 'Error'}
     </div>

src/createBaseRouter.js

@@ -38,6 +38,9 @@ export default function createBaseRouter({ routeConfig, matcher, render }) {
       };
 
       this.mounted = true;
+
+      this.shouldResolveMatch = false;
+      this.pendingResolvedMatch = false;
     }
 
     getChildContext() {
@@ -73,12 +76,19 @@ export default function createBaseRouter({ routeConfig, matcher, render }) {
       }
     }
 
-    componentDidUpdate(prevProps) {
+    componentWillReceiveProps(nextProps) {
       if (
-        this.props.match !== prevProps.match ||
-        this.props.resolveElements !== prevProps.resolveElements ||
-        !isEqual(this.props.matchContext, prevProps.matchContext)
+        nextProps.match !== this.props.match ||
+        nextProps.resolveElements !== this.props.resolveElements ||
+        !isEqual(nextProps.matchContext, this.props.matchContext)
       ) {
+        this.shouldResolveMatch = true;
+      }
+    }
+
+    componentDidUpdate() {
+      if (this.shouldResolveMatch) {
+        this.shouldResolveMatch = false;
         this.resolveMatch();
       }
     }
@@ -113,10 +123,6 @@ export default function createBaseRouter({ routeConfig, matcher, render }) {
             return;
           }
 
-          if (!elements) {
-            continue; // eslint-disable-line no-continue
-          }
-
           this.updateElement({ ...augmentedMatch, elements });
         }
       } catch (e) {
@@ -143,26 +149,37 @@ export default function createBaseRouter({ routeConfig, matcher, render }) {
     }
 
     updateElement(renderArgs) {
-      this.setState({ element: render(renderArgs) });
-
       const { resolvedMatch, match } = this.props;
 
-      if (resolvedMatch !== match) {
+      // If we're about to mark the match resolved, delay the rerender until we
+      // do so.
+      this.pendingResolvedMatch = !!(
+        (renderArgs.elements || renderArgs.error) &&
+        resolvedMatch !== match
+      );
+
+      this.setState({ element: render(renderArgs) });
+
+      if (this.pendingResolvedMatch) {
         // If this is a new match, update the store, so we can rerender at the
         // same time as all of the links and other components connected to the
         // router state.
+        this.pendingResolvedMatch = false;
         this.props.onResolveMatch(match);
       }
     }
 
     render() {
-      const { resolvedMatch, match } = this.props;
-
-      // Normally, returning the same ReactElement is sufficient to skip
-      // reconciliation. However, that doesn't work with context. Additionally,
-      // we only need to block rerendering while a match is pending anyway.
+      // Don't rerender synchronously if we have another rerender coming. Just
+      // memoizing the element here doesn't do anything because we're using
+      // context.
       return (
-        <StaticContainer shouldUpdate={resolvedMatch === match}>
+        <StaticContainer
+          shouldUpdate={
+            !this.shouldResolveMatch &&
+            !this.pendingResolvedMatch
+          }
+        >
           {this.state.element}
         </StaticContainer>
       );

src/createBrowserRouter.js

@@ -7,13 +7,13 @@ import createRender from './createRender';
 import resolveElements from './resolveElements';
 
 export default function createBrowserRouter({
-  basename, renderError, ...options
+  basename, renderPending, renderReady, renderError, ...options
 }) {
   const FarceRouter = createFarceRouter({
     ...options,
     historyProtocol: new BrowserProtocol({ basename }),
     historyMiddlewares: [queryMiddleware],
-    render: createRender({ renderError }),
+    render: createRender({ renderPending, renderReady, renderError }),
   });
 
   function BrowserRouter(props) {

src/createConnectedRouter.js

@@ -28,8 +28,8 @@ export default function createConnectedRouter({
     },
     null,
     {
-      // The top-level router shouldn't block context propagation from above.
-      // It should seldom be rerendering anyway.
+      // Don't block context propagation from above. The router should seldom
+      // be unnecessarily rerendering anyway.
       pure: false,
     },
   )(createBaseRouter(options));

src/createRender.js

@@ -1,16 +1,33 @@
 import React from 'react';
+import StaticContainer from 'react-static-container';
 
 import ElementsRenderer from './ElementsRenderer';
 
-// TODO: There should probably be a renderLoading or something here.
-export default function createRender({ renderError }) {
-  // This is not a component.
-  // eslint-disable-next-line react/prop-types
-  return function render({ error, elements }) {
+// These are intentionally not renderLoading, renderFetched, and renderFailure
+// from Relay, because these don't quite correspond to those conditions.
+export default function createRender({
+  renderPending, renderReady, renderError,
+}) {
+  return function render(renderArgs) {
+    const { error, elements } = renderArgs;
+    let element;
+
     if (error) {
-      return renderError ? renderError(error) : null;
+      element = renderError ? renderError(renderArgs) : null;
+    } else if (!elements) {
+      element = renderPending ? renderPending(renderArgs) : undefined;
+    } else if (renderReady) {
+      element = renderReady(renderArgs);
+    } else {
+      element = <ElementsRenderer elements={elements} />;
     }
 
-    return <ElementsRenderer elements={elements} />;
+    const hasElement = element !== undefined;
+
+    return (
+      <StaticContainer shouldUpdate={hasElement}>
+        {hasElement ? element : null}
+      </StaticContainer>
+    );
   };
 }