Browse Source

ref(routes): Simplify routing tree using custom route builder (#66747)

This helps reduce the current complexity of reading the route tree by
introducing some logic which builds the two org and orgless routes from
a single route.

This only changes the simplest cases. There are a number of more
complicated routes that can also be simplified that I will do in a
follow up.
Evan Purkhiser 1 year ago
parent
commit
a936fb00da
1 changed files with 157 additions and 294 deletions
  1. 157 294
      static/app/routes.tsx

+ 157 - 294
static/app/routes.tsx

@@ -1,5 +1,5 @@
 import {Fragment} from 'react';
-import type {IndexRouteProps, RouteProps} from 'react-router';
+import type {IndexRouteProps, PlainRoute, RouteProps} from 'react-router';
 import {
   IndexRedirect,
   IndexRoute as BaseIndexRoute,
@@ -30,16 +30,84 @@ import RouteNotFound from 'sentry/views/routeNotFound';
 import SettingsWrapper from 'sentry/views/settings/components/settingsWrapper';
 
 type CustomProps = {
+  /**
+   * Human readable route name. This is primarily used in the settings routes.
+   */
   name?: string;
+  /**
+   * Ensure this route renders two routes, one for the "org" path which
+   * includes the :orgId slug, and one without.
+   *
+   * Setting this to `true` will prefix the provided path of the secondary
+   * route with:
+   *
+   *   /organizations/:orgId
+   *
+   * Setting this will wrap the two routes in withDomainRequired and
+   * withDomainRedirect respectively.
+   */
+  withOrgPath?: boolean;
 };
 
+interface SentryRouteProps extends React.PropsWithChildren<RouteProps & CustomProps> {}
+
 /**
- * We add some additional props to our routes
+ * Customized React Router Route configuration component.
  */
+const Route = BaseRoute as React.ComponentClass<SentryRouteProps>;
+
+// The original createRouteFromReactElement extracted from the base route. This
+// is not properly typed hence the ts-ignore.
+//
+// @ts-ignore
+const createRouteFromReactElement = BaseRoute.createRouteFromReactElement;
+
+type RouteElement = React.ReactElement<SentryRouteProps>;
+
+// We override the createRouteFromReactElement property to provide support for
+// the withOrgPath property.
+//
+// XXX(epurkhiser): It is important to note! The `Route` component is a
+// CONFIGURATION ONLY COMPONENT. It DOES NOT render! This function is part of
+// the react-router magic internals that are used to build the route tree by
+// traversing the component tree, that is why this logic lives here and not
+// inside a custom Route component.
+//
+// To understand deeper how this works, see [0].
+//
+// When `withOrgPath` is provided to the Route configuration component the
+// react-router router builder will use this function which splits the single
+// Route into two, one for the route with :orgId and one for the new-style
+// route.
+//
+// [0]: https://github.com/remix-run/react-router/blob/850de933444d260bfc5460135d308f9d74b52c97/modules/RouteUtils.js#L15
+//
+// @ts-ignore
+Route.createRouteFromReactElement = function (element: RouteElement): PlainRoute {
+  const {withOrgPath, component, path} = element.props;
 
-const Route = BaseRoute as React.ComponentClass<
-  React.PropsWithChildren<RouteProps & CustomProps>
->;
+  if (!withOrgPath) {
+    return createRouteFromReactElement(element);
+  }
+
+  const childRoutes: PlainRoute[] = [
+    {
+      ...createRouteFromReactElement(element),
+      path: `/organizations/:orgId${path}`,
+      component: withDomainRedirect(component ?? NoOp),
+    },
+  ];
+
+  if (USING_CUSTOMER_DOMAIN) {
+    childRoutes.push({
+      ...createRouteFromReactElement(element),
+      path,
+      component: withDomainRequired(component ?? NoOp),
+    });
+  }
+
+  return {childRoutes};
+};
 
 const IndexRoute = BaseIndexRoute as React.ComponentClass<IndexRouteProps & CustomProps>;
 
@@ -133,12 +201,9 @@ function buildRoutes() {
   //   loads the organiztion into context (though in some cases, there may be
   //   no organiztion)
   //
-  //   When adding new routes make sure you have both a route that starts
-  //   with `/organizations/:orgId` and also 'customer-domains' route that
-  //   does not include `/organizations/:orgId`. Often you'll only need to
-  //   worry about this for the container route for that section of the UI.
-  //   Child routes should access the current organization with `useOrganization()`
-  //   or `withOrganization()` methods.
+  //   When adding new top-level organization routes, be sure the top level
+  //   route includes withOrgPath to support installs that are not using
+  //   customer domains.
   //
   //   Within these routes are a variety of subroutes. They are not all
   //   listed here as the subroutes will be added and removed, and most are
@@ -1083,8 +1148,12 @@ function buildRoutes() {
     </Route>
   );
 
-  const projectsChildRoutes = (
-    <Fragment>
+  const projectsRoutes = (
+    <Route
+      path="/projects/"
+      component={make(() => import('sentry/views/projects/'))}
+      withOrgPath
+    >
       <IndexRoute component={make(() => import('sentry/views/projectsDashboard'))} />
       <Route
         path="new/"
@@ -1104,27 +1173,7 @@ function buildRoutes() {
           () => import('sentry/views/projectInstall/platformOrIntegration')
         )}
       />
-    </Fragment>
-  );
-  const projectsRoutes = (
-    <Fragment>
-      {USING_CUSTOMER_DOMAIN && (
-        <Route
-          path="/projects/"
-          component={make(() => import('sentry/views/projects/'))}
-          key="orgless-projects-route"
-        >
-          {projectsChildRoutes}
-        </Route>
-      )}
-      <Route
-        path="/organizations/:orgId/projects/"
-        component={make(() => import('sentry/views/projects/'))}
-        key="org-projects"
-      >
-        {projectsChildRoutes}
-      </Route>
-    </Fragment>
+    </Route>
   );
 
   const dashboardWidgetRoutes = (
@@ -1387,8 +1436,12 @@ function buildRoutes() {
     </Fragment>
   );
 
-  const cronsChildRoutes = (
-    <Fragment>
+  const cronsRoutes = (
+    <Route
+      path="/crons/"
+      component={make(() => import('sentry/views/monitors'))}
+      withOrgPath
+    >
       <IndexRoute component={make(() => import('sentry/views/monitors/overview'))} />
       <Route
         path="create/"
@@ -1402,31 +1455,15 @@ function buildRoutes() {
         path=":monitorSlug/edit/"
         component={make(() => import('sentry/views/monitors/edit'))}
       />
-    </Fragment>
-  );
-  const cronsRoutes = (
-    <Fragment>
-      {USING_CUSTOMER_DOMAIN && (
-        <Route
-          path="/crons/"
-          component={withDomainRequired(make(() => import('sentry/views/monitors')))}
-          key="orgless-monitors-route"
-        >
-          {cronsChildRoutes}
-        </Route>
-      )}
-      <Route
-        path="/organizations/:orgId/crons/"
-        component={withDomainRedirect(make(() => import('sentry/views/monitors')))}
-        key="org-monitors"
-      >
-        {cronsChildRoutes}
-      </Route>
-    </Fragment>
+    </Route>
   );
 
-  const replayChildRoutes = (
-    <Fragment>
+  const replayRoutes = (
+    <Route
+      path="/replays/"
+      component={make(() => import('sentry/views/replays/index'))}
+      withOrgPath
+    >
       <IndexRoute component={make(() => import('sentry/views/replays/list'))} />
       <Route
         path="selectors/"
@@ -1438,27 +1475,7 @@ function buildRoutes() {
         path=":replaySlug/"
         component={make(() => import('sentry/views/replays/details'))}
       />
-    </Fragment>
-  );
-  const replayRoutes = (
-    <Fragment>
-      {USING_CUSTOMER_DOMAIN && (
-        <Route
-          path="/replays/"
-          component={withDomainRequired(make(() => import('sentry/views/replays/index')))}
-          key="orgless-replays-route"
-        >
-          {replayChildRoutes}
-        </Route>
-      )}
-      <Route
-        path="/organizations/:orgId/replays/"
-        component={withDomainRedirect(make(() => import('sentry/views/replays/index')))}
-        key="org-replays"
-      >
-        {replayChildRoutes}
-      </Route>
-    </Fragment>
+    </Route>
   );
 
   const releasesChildRoutes = ({forCustomerDomain}: {forCustomerDomain: boolean}) => {
@@ -1521,49 +1538,19 @@ function buildRoutes() {
     </Fragment>
   );
   const releaseThresholdRoutes = (
-    <Fragment>
-      {USING_CUSTOMER_DOMAIN && (
-        <Route
-          path="/release-thresholds/"
-          component={withDomainRequired(NoOp)}
-          key="orgless-release-thresholds-route"
-        >
-          <IndexRoute
-            component={make(() => import('sentry/views/releases/thresholdsList'))}
-          />
-        </Route>
-      )}
-      <Route
-        path="/organizations/:orgId/release-thresholds/"
-        component={withDomainRedirect(NoOp)}
-        key="org-release-thresholds"
-      >
-        <IndexRoute
-          component={make(() => import('sentry/views/releases/thresholdsList'))}
-        />
-      </Route>
-    </Fragment>
+    <Route path="/release-thresholds/" withOrgPath>
+      <IndexRoute
+        component={make(() => import('sentry/views/releases/thresholdsList'))}
+      />
+    </Route>
   );
 
   const activityRoutes = (
-    <Fragment>
-      {USING_CUSTOMER_DOMAIN && (
-        <Route
-          path="/activity/"
-          component={withDomainRequired(
-            make(() => import('sentry/views/organizationActivity'))
-          )}
-          key="orgless-activity-route"
-        />
-      )}
-      <Route
-        path="/organizations/:orgId/activity/"
-        component={withDomainRedirect(
-          make(() => import('sentry/views/organizationActivity'))
-        )}
-        key="org-activity"
-      />
-    </Fragment>
+    <Route
+      path="/activity/"
+      component={make(() => import('sentry/views/organizationActivity'))}
+      withOrgPath
+    />
   );
 
   const statsChildRoutes = ({forCustomerDomain}: {forCustomerDomain: boolean}) => {
@@ -1621,8 +1608,12 @@ function buildRoutes() {
   // should be the canonical route for discover2. We have a redirect right now
   // as /discover was for discover 1 and most of the application is linking to
   // /discover/queries and not /discover
-  const discoverChildRoutes = (
-    <Fragment>
+  const discoverRoutes = (
+    <Route
+      path="/discover/"
+      component={make(() => import('sentry/views/discover'))}
+      withOrgPath
+    >
       <IndexRedirect to="queries/" />
       <Route
         path="homepage/"
@@ -1640,31 +1631,15 @@ function buildRoutes() {
         path=":eventSlug/"
         component={make(() => import('sentry/views/discover/eventDetails'))}
       />
-    </Fragment>
-  );
-  const discoverRoutes = (
-    <Fragment>
-      {USING_CUSTOMER_DOMAIN && (
-        <Route
-          path="/discover/"
-          component={withDomainRequired(make(() => import('sentry/views/discover')))}
-          key="orgless-discover-route"
-        >
-          {discoverChildRoutes}
-        </Route>
-      )}
-      <Route
-        path="/organizations/:orgId/discover/"
-        component={withDomainRedirect(make(() => import('sentry/views/discover')))}
-        key="org-discover-route"
-      >
-        {discoverChildRoutes}
-      </Route>
-    </Fragment>
+    </Route>
   );
 
-  const performanceChildRoutes = (
-    <Fragment>
+  const performanceRoutes = (
+    <Route
+      path="/performance/"
+      component={make(() => import('sentry/views/performance'))}
+      withOrgPath
+    >
       <IndexRoute component={make(() => import('sentry/views/performance/content'))} />
       <Route
         path="trends/"
@@ -1848,31 +1823,15 @@ function buildRoutes() {
         path=":eventSlug/"
         component={make(() => import('sentry/views/performance/transactionDetails'))}
       />
-    </Fragment>
-  );
-  const performanceRoutes = (
-    <Fragment>
-      {USING_CUSTOMER_DOMAIN && (
-        <Route
-          path="/performance/"
-          component={withDomainRequired(make(() => import('sentry/views/performance')))}
-          key="orgless-performance-route"
-        >
-          {performanceChildRoutes}
-        </Route>
-      )}
-      <Route
-        path="/organizations/:orgId/performance/"
-        component={withDomainRedirect(make(() => import('sentry/views/performance')))}
-        key="org-performance"
-      >
-        {performanceChildRoutes}
-      </Route>
-    </Fragment>
+    </Route>
   );
 
-  const starfishChildRoutes = (
-    <Fragment>
+  const starfishRoutes = (
+    <Route
+      path="/starfish/"
+      component={make(() => import('sentry/views/starfish'))}
+      withOrgPath
+    >
       <IndexRoute
         component={make(() => import('sentry/views/starfish/views/webServiceView'))}
       />
@@ -1915,96 +1874,37 @@ function buildRoutes() {
           component={make(() => import('sentry/views/starfish/views/spanSummaryPage'))}
         />
       </Route>
-    </Fragment>
-  );
-  const starfishRoutes = (
-    <Fragment>
-      {USING_CUSTOMER_DOMAIN && (
-        <Route
-          path="/starfish/"
-          component={withDomainRequired(make(() => import('sentry/views/starfish')))}
-          key="orgless-starfish-route"
-        >
-          {starfishChildRoutes}
-        </Route>
-      )}
-      <Route
-        path="/organizations/:orgId/starfish/"
-        component={withDomainRedirect(make(() => import('sentry/views/starfish/')))}
-        key="org-starfish"
-      >
-        {starfishChildRoutes}
-      </Route>
-    </Fragment>
+    </Route>
   );
 
   const userFeedbackRoutes = (
-    <Fragment>
-      {USING_CUSTOMER_DOMAIN && (
-        <Route
-          path="/user-feedback/"
-          component={withDomainRequired(make(() => import('sentry/views/userFeedback')))}
-          key="orgless-user-feedback-route"
-        />
-      )}
-      <Route
-        path="/organizations/:orgId/user-feedback/"
-        component={withDomainRedirect(make(() => import('sentry/views/userFeedback')))}
-        key="org-user-feedback"
-      />
-    </Fragment>
+    <Route
+      path="/user-feedback/"
+      component={make(() => import('sentry/views/userFeedback'))}
+      withOrgPath
+    />
   );
 
-  const feedbackChildRoutes = (
-    <Fragment>
+  const feedbackv2Routes = (
+    <Route
+      path="/feedback/"
+      component={make(() => import('sentry/views/feedback/index'))}
+      withOrgPath
+    >
       <IndexRoute
         component={make(() => import('sentry/views/feedback/feedbackListPage'))}
       />
-    </Fragment>
-  );
-  const feedbackv2Routes = (
-    <Fragment>
-      {USING_CUSTOMER_DOMAIN && (
-        <Route
-          path="/feedback/"
-          component={withDomainRequired(
-            make(() => import('sentry/views/feedback/index'))
-          )}
-          key="orgless-feedback-list-route"
-        >
-          {feedbackChildRoutes}
-        </Route>
-      )}
-      <Route
-        path="/organizations/:orgId/feedback/"
-        component={withDomainRedirect(make(() => import('sentry/views/feedback/index')))}
-        key="org-feedback-list-route"
-      >
-        {feedbackChildRoutes}
-      </Route>
-    </Fragment>
+    </Route>
   );
 
   const issueListRoutes = (
-    <Fragment>
-      {USING_CUSTOMER_DOMAIN && (
-        <Route
-          path="/issues/(searches/:searchId/)"
-          component={withDomainRequired(errorHandler(IssueListContainer))}
-          key="orgless-issues-route"
-        >
-          <IndexRoute component={errorHandler(IssueListOverview)} />
-        </Route>
-      )}
-      <Route
-        path="/organizations/:orgId/issues/(searches/:searchId/)"
-        component={withDomainRedirect(errorHandler(IssueListContainer))}
-        key="org-issues"
-      >
-        <Redirect from="/organizations/:orgId/" to="/organizations/:orgId/issues/" />
-        <IndexRoute component={errorHandler(IssueListOverview)} />
-      </Route>
-    </Fragment>
+    <Route
+      path="/issues/(searches/:searchId/)"
+      component={errorHandler(IssueListContainer)}
+      withOrgPath
+    >
+      <IndexRoute component={errorHandler(IssueListOverview)} />
+    </Route>
   );
 
   // Once org issues is complete, these routes can be nested under
@@ -2238,8 +2138,12 @@ function buildRoutes() {
     </Fragment>
   );
 
-  const profilingChildRoutes = (
-    <Fragment>
+  const profilingRoutes = (
+    <Route
+      path="/profiling/"
+      component={make(() => import('sentry/views/profiling'))}
+      withOrgPath
+    >
       <IndexRoute component={make(() => import('sentry/views/profiling/content'))} />
       <Route
         path="summary/:projectId/"
@@ -2258,54 +2162,13 @@ function buildRoutes() {
           component={make(() => import('sentry/views/profiling/profileFlamechart'))}
         />
       </Route>
-    </Fragment>
-  );
-  const profilingRoutes = (
-    <Fragment>
-      {USING_CUSTOMER_DOMAIN && (
-        <Route
-          path="/profiling/"
-          component={withDomainRequired(make(() => import('sentry/views/profiling')))}
-          key="orgless-profiling-route"
-        >
-          {profilingChildRoutes}
-        </Route>
-      )}
-      <Route
-        path="/organizations/:orgId/profiling/"
-        component={withDomainRedirect(make(() => import('sentry/views/profiling')))}
-        key="org-profiling"
-      >
-        {profilingChildRoutes}
-      </Route>
-    </Fragment>
-  );
-
-  const ddmChildRoutes = (
-    <Fragment>
-      <IndexRoute component={make(() => import('sentry/views/ddm/ddm'))} />
-    </Fragment>
+    </Route>
   );
 
   const ddmRoutes = (
-    <Fragment>
-      {USING_CUSTOMER_DOMAIN && (
-        <Route
-          path="/ddm/"
-          component={withDomainRequired(make(() => import('sentry/views/ddm')))}
-          key="orgless-ddm-route"
-        >
-          {ddmChildRoutes}
-        </Route>
-      )}
-      <Route
-        path="/organizations/:orgId/ddm/"
-        component={withDomainRedirect(make(() => import('sentry/views/ddm')))}
-        key="org-ddm"
-      >
-        {ddmChildRoutes}
-      </Route>
-    </Fragment>
+    <Route path="/ddm/" component={make(() => import('sentry/views/ddm'))} withOrgPath>
+      <IndexRoute component={make(() => import('sentry/views/ddm/ddm'))} />
+    </Route>
   );
 
   // Support for deprecated URLs (pre-Sentry 10). We just redirect users to new