Browse Source

ref(js): Avoid react-router link in favor of ours (#28033)

Evan Purkhiser 3 years ago
parent
commit
c1194cfda3

+ 1 - 1
static/app/components/button.tsx

@@ -1,10 +1,10 @@
 import * as React from 'react';
-import {Link} from 'react-router';
 import isPropValid from '@emotion/is-prop-valid';
 import {css} from '@emotion/react';
 import styled from '@emotion/styled';
 
 import ExternalLink from 'app/components/links/externalLink';
+import Link from 'app/components/links/link';
 import Tooltip from 'app/components/tooltip';
 import mergeRefs from 'app/utils/mergeRefs';
 import {Theme} from 'app/utils/theme';

+ 1 - 1
static/app/components/errorRobot.tsx

@@ -1,11 +1,11 @@
 import {Component} from 'react';
-import {Link} from 'react-router';
 import styled from '@emotion/styled';
 
 import robotBackground from 'sentry-images/spot/sentry-robot.png';
 
 import {Client} from 'app/api';
 import Button from 'app/components/button';
+import Link from 'app/components/links/link';
 import {t, tct} from 'app/locale';
 import space from 'app/styles/space';
 import {LightWeightOrganization, Project} from 'app/types';

+ 2 - 1
static/app/components/eventOrGroupExtraDetails.tsx

@@ -1,4 +1,4 @@
-import {Link, withRouter, WithRouterProps} from 'react-router';
+import {withRouter, WithRouterProps} from 'react-router';
 import styled from '@emotion/styled';
 
 import GuideAnchor from 'app/components/assistant/guideAnchor';
@@ -8,6 +8,7 @@ import InboxShortId from 'app/components/group/inboxBadges/shortId';
 import TimesTag from 'app/components/group/inboxBadges/timesTag';
 import UnhandledTag from 'app/components/group/inboxBadges/unhandledTag';
 import ProjectBadge from 'app/components/idBadge/projectBadge';
+import Link from 'app/components/links/link';
 import Placeholder from 'app/components/placeholder';
 import {IconChat} from 'app/icons';
 import {tct} from 'app/locale';

+ 1 - 1
static/app/components/events/eventTags/eventTagsPill.tsx

@@ -1,4 +1,3 @@
-import {Link} from 'react-router';
 import {css} from '@emotion/react';
 import {Query} from 'history';
 import * as queryString from 'query-string';
@@ -6,6 +5,7 @@ import * as queryString from 'query-string';
 import AnnotatedText from 'app/components/events/meta/annotatedText';
 import {getMeta} from 'app/components/events/meta/metaProxy';
 import ExternalLink from 'app/components/links/externalLink';
+import Link from 'app/components/links/link';
 import Pill from 'app/components/pill';
 import VersionHoverCard from 'app/components/versionHoverCard';
 import {IconInfo, IconOpen} from 'app/icons';

+ 26 - 29
static/app/components/globalSelectionLink.tsx

@@ -1,8 +1,9 @@
 import * as React from 'react';
-import {Link as RouterLink, withRouter, WithRouterProps} from 'react-router';
+import {withRouter, WithRouterProps} from 'react-router';
 import {LocationDescriptor} from 'history';
 import * as qs from 'query-string';
 
+import Link from 'app/components/links/link';
 import {extractSelectionParameters} from 'app/components/organizations/globalSelectionHeader/utils';
 
 type Props = WithRouterProps & {
@@ -21,7 +22,7 @@ type Props = WithRouterProps & {
   /**
    * Click event (not for navigation)
    */
-  onClick?: React.ComponentProps<typeof RouterLink>['onClick'];
+  onClick?: React.ComponentProps<typeof Link>['onClick'];
 };
 
 /**
@@ -41,41 +42,37 @@ class GlobalSelectionLink extends React.Component<Props> {
       typeof to === 'object' && to.query ? {...globalQuery, ...to.query} : globalQuery;
 
     if (location) {
-      let toWithGlobalQuery;
-      if (hasGlobalQuery) {
-        if (typeof to === 'string') {
-          toWithGlobalQuery = {pathname: to, query};
-        } else {
-          toWithGlobalQuery = {...to, query};
-        }
-      }
+      const toWithGlobalQuery: LocationDescriptor = hasGlobalQuery
+        ? typeof to === 'string'
+          ? {pathname: to, query}
+          : {...to, query}
+        : {};
+
       const routerProps = hasGlobalQuery
         ? {...this.props, to: toWithGlobalQuery}
         : {...this.props, to};
 
-      return <RouterLink {...routerProps}>{this.props.children}</RouterLink>;
-    } else {
-      let queryStringObject = {};
-      if (typeof to === 'object' && to.search) {
-        queryStringObject = qs.parse(to.search);
-      }
+      return <Link {...routerProps}>{this.props.children}</Link>;
+    }
 
-      queryStringObject = {...queryStringObject, ...globalQuery};
+    let queryStringObject = {};
+    if (typeof to === 'object' && to.search) {
+      queryStringObject = qs.parse(to.search);
+    }
 
-      if (typeof to === 'object' && to.query) {
-        queryStringObject = {...queryStringObject, ...to.query};
-      }
+    queryStringObject = {...queryStringObject, ...globalQuery};
 
-      const url =
-        (typeof to === 'string' ? to : to.pathname) +
-        '?' +
-        qs.stringify(queryStringObject);
-      return (
-        <a {...this.props} href={url}>
-          {this.props.children}
-        </a>
-      );
+    if (typeof to === 'object' && to.query) {
+      queryStringObject = {...queryStringObject, ...to.query};
     }
+
+    const url =
+      (typeof to === 'string' ? to : to.pathname) + '?' + qs.stringify(queryStringObject);
+    return (
+      <a {...this.props} href={url}>
+        {this.props.children}
+      </a>
+    );
   }
 }
 

+ 1 - 1
static/app/components/issueLink.tsx

@@ -1,5 +1,4 @@
 import * as React from 'react';
-import {Link} from 'react-router';
 import styled from '@emotion/styled';
 import classNames from 'classnames';
 
@@ -8,6 +7,7 @@ import EventOrGroupTitle from 'app/components/eventOrGroupTitle';
 import EventAnnotation from 'app/components/events/eventAnnotation';
 import EventMessage from 'app/components/events/eventMessage';
 import Hovercard from 'app/components/hovercard';
+import Link from 'app/components/links/link';
 import TimeSince from 'app/components/timeSince';
 import {t} from 'app/locale';
 import overflowEllipsis from 'app/styles/overflowEllipsis';

+ 25 - 19
static/app/components/links/link.tsx

@@ -1,4 +1,4 @@
-import * as React from 'react';
+import {useEffect} from 'react';
 import {Link as RouterLink, withRouter, WithRouterProps} from 'react-router';
 import isPropValid from '@emotion/is-prop-valid';
 import styled from '@emotion/styled';
@@ -10,9 +10,13 @@ type AnchorProps = React.HTMLProps<HTMLAnchorElement>;
 type ToLocationFunction = (location: Location) => LocationDescriptor;
 
 type Props = WithRouterProps & {
-  // URL
+  /**
+   * The string path or LocationDescriptor object
+   */
   to: ToLocationFunction | LocationDescriptor;
-  // Styles applied to the component's root
+  /**
+   * Style applied to the component's root
+   */
   className?: string;
 } & Omit<AnchorProps, 'href' | 'target' | 'as' | 'css'>;
 
@@ -20,32 +24,34 @@ type Props = WithRouterProps & {
  * A context-aware version of Link (from react-router) that falls
  * back to <a> if there is no router present
  */
-class Link extends React.Component<Props> {
-  componentDidMount() {
-    const isRouterPresent = this.props.location;
-    if (!isRouterPresent) {
+const BaseLink: React.FC<Props> = ({location, disabled, to, ref, ...props}) => {
+  useEffect(() => {
+    // check if the router is present
+    if (!location) {
       Sentry.captureException(
         new Error('The link component was rendered without being wrapped by a <Router />')
       );
     }
+  }, []);
+
+  if (!disabled && location) {
+    return <RouterLink to={to} ref={ref as any} {...props} />;
   }
 
-  render() {
-    const {disabled, to, ref, location, ...props} = this.props;
+  if (typeof to === 'string') {
+    return <Anchor href={to} ref={ref} disabled={disabled} {...props} />;
+  }
 
-    if (!disabled && location) {
-      return <RouterLink to={to} ref={ref as any} {...props} />;
-    }
+  return <Anchor href="" ref={ref} {...props} disabled />;
+};
 
-    if (typeof to === 'string') {
-      return <Anchor href={to} ref={ref} disabled={disabled} {...props} />;
-    }
+// Set the displayName for testing convenience
+BaseLink.displayName = 'Link';
 
-    return <Anchor href="" ref={ref} {...props} disabled />;
-  }
-}
+// Re-assign to Link to make auto-importing smarter
+const Link = withRouter(BaseLink);
 
-export default withRouter(Link);
+export default Link;
 
 const Anchor = styled('a', {
   shouldForwardProp: prop =>

+ 4 - 4
static/app/components/links/listLink.tsx

@@ -1,5 +1,5 @@
 import * as React from 'react';
-import {Link, withRouter, WithRouterProps} from 'react-router';
+import {Link as RouterLink, withRouter, WithRouterProps} from 'react-router';
 import styled from '@emotion/styled';
 import classNames from 'classnames';
 import {LocationDescriptor} from 'history';
@@ -12,7 +12,7 @@ type DefaultProps = {
   disabled: boolean;
 };
 
-type LinkProps = Omit<React.ComponentProps<typeof Link>, 'to'>;
+type LinkProps = Omit<React.ComponentProps<typeof RouterLink>, 'to'>;
 
 type Props = WithRouterProps &
   Partial<DefaultProps> &
@@ -79,9 +79,9 @@ class ListLink extends React.Component<Props> {
 
     return (
       <StyledLi className={this.getClassName()} disabled={disabled}>
-        <Link {...carriedProps} onlyActiveOnIndex={index} to={disabled ? '' : to}>
+        <RouterLink {...carriedProps} onlyActiveOnIndex={index} to={disabled ? '' : to}>
           {children}
-        </Link>
+        </RouterLink>
       </StyledLi>
     );
   }

+ 1 - 1
static/app/components/organizations/backToIssues.tsx

@@ -1,6 +1,6 @@
-import {Link} from 'react-router';
 import styled from '@emotion/styled';
 
+import Link from 'app/components/links/link';
 import space from 'app/styles/space';
 
 const BackToIssues = styled(Link)`

+ 1 - 1
static/app/components/organizations/headerItem.tsx

@@ -1,9 +1,9 @@
 import * as React from 'react';
-import {Link} from 'react-router';
 import isPropValid from '@emotion/is-prop-valid';
 import styled from '@emotion/styled';
 import omit from 'lodash/omit';
 
+import Link from 'app/components/links/link';
 import Tooltip from 'app/components/tooltip';
 import {IconChevron, IconClose, IconInfo, IconLock, IconSettings} from 'app/icons';
 import {t} from 'app/locale';

Some files were not shown because too many files changed in this diff