fix: Sweeping fix for React key usage (#473)

Fixes a bunch of React issues, mainly `key` usage but also some errors around `useEffect` dependencies and some other best practices.
getsentry · Aug 6, 2024 · d38c054 · d38c054
1 parent 125d8b0
commit d38c054
Show file tree

Hide file tree

Showing 35 changed files with 604 additions and 690 deletions.
diff --git a/.changeset/light-socks-admire.md b/.changeset/light-socks-admire.md
@@ -0,0 +1,7 @@
+---
+'@spotlightjs/overlay': patch
+'@spotlightjs/electron': patch
+'@spotlightjs/spotlight': patch
+---
+
+Overhaul React code for multiple fixes and performance improvements
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -9,6 +9,8 @@
     "spotlightjs",
     "svgr",
     "tailwindcss",
-    "uuidv"
+    "ttfb",
+    "uuidv",
+    "webvitals"
   ]
 }
diff --git a/packages/overlay/src/components/Overview.tsx b/packages/overlay/src/components/Overview.tsx
@@ -51,21 +51,12 @@ export default function Overview({
       <Tabs tabs={tabs} setOpen={setOpen} />
       <div className="flex-1 overflow-auto overflow-x-hidden">
         <Routes>
-          <Route path="/not-found" element={<p>Not Found - How'd you manage to get here?</p>} key={'not-found'}></Route>
-          {tabs.map(tab => {
-            const TabContent = tab.content && tab.content;
-
-            if (TabContent) {
-              return (
-                <Route
-                  path={`/${tab.id}/*`}
-                  key={tab.id}
-                  element={createElement(TabContent, { processedEvents: tab.processedEvents })}
-                ></Route>
-              );
-            }
-            return null;
-          })}
+          <Route path="/not-found" element={<p>Not Found - How'd you manage to get here?</p>} key={'not-found'} />
+          {tabs.map(({ content: TabContent, id, processedEvents }) =>
+            TabContent ? (
+              <Route path={`/${id}/*`} key={id} element={createElement(TabContent, { processedEvents })} />
+            ) : null,
+          )}
         </Routes>
       </div>
     </>

diff --git a/packages/overlay/src/components/Tabs.tsx b/packages/overlay/src/components/Tabs.tsx
@@ -55,14 +55,14 @@ export default function Tabs({ tabs, nested, setOpen }: Props) {
           className="border-primary-800 bg-primary-800 hover:bg-primary-700 hover:border-primary-700 focus:bg-primary-800 text-primary-100 block w-full rounded-md py-2 pl-3 pr-10 focus:outline-none sm:text-sm"
           onChange={e => {
             const activeTab = tabs.find(tab => tab.id === e.target.value);
-            if (activeTab && activeTab.onSelect) {
+            if (activeTab?.onSelect) {
               activeTab.onSelect();
             }
             navigate(`${nested ? '' : '/'}${activeTab?.id || 'not-found'}`);
           }}
         >
-          {tabs.map((tab, tabIdx) => (
-            <option key={tabIdx} value={tab.id}>
+          {tabs.map(tab => (
+            <option key={tab.id} value={tab.id}>
               {tab.title} {tab.notificationCount?.count}
             </option>
           ))}
@@ -83,7 +83,7 @@ export default function Tabs({ tabs, nested, setOpen }: Props) {
                   '-m-y -mx-2 flex select-none whitespace-nowrap border-b-2 px-2 py-3 text-sm font-medium',
                 )
               }
-              onClick={() => tab.onSelect && tab.onSelect()}
+              onClick={() => tab.onSelect?.()}
             >
               {tab.title}
               {tab.notificationCount !== undefined ? (

diff --git a/packages/overlay/src/components/Trigger.tsx b/packages/overlay/src/components/Trigger.tsx
@@ -16,7 +16,7 @@ function getAnchorClasses(anchor: AnchorConfig) {
       return 'top-4 right-4';
     case 'bottomLeft':
       return 'bottom-4 left-4';
-    case 'bottomRight':
+    // case 'bottomRight':
     default:
       return 'bottom-4 right-4';
   }
@@ -35,7 +35,7 @@ function ToolbarItem({
     <div className="gap-x hover:bg-primary-400 relative flex items-center rounded p-3" {...props}>
       {children}
 
-      {!!count && (
+      {count && (
         <span
           className={classNames(
             severe ? 'bg-red-500' : 'bg-primary-500',

diff --git a/packages/overlay/src/integrations/console/console-tab.tsx b/packages/overlay/src/integrations/console/console-tab.tsx
@@ -1,4 +1,4 @@
-import { ConsoleMessage } from './types';
+import type { ConsoleMessage } from './types';
 
 type Props = {
   processedEvents?: ConsoleMessage[];

diff --git a/packages/overlay/src/integrations/sentry/components/HiddenItemsButton.tsx b/packages/overlay/src/integrations/sentry/components/HiddenItemsButton.tsx
@@ -1,4 +1,4 @@
-import { type ComponentProps } from 'react';
+import type { ComponentProps } from 'react';
 
 export default function HiddenItemsButton({
   itemCount,

diff --git a/packages/overlay/src/integrations/sentry/components/SdkList.tsx b/packages/overlay/src/integrations/sentry/components/SdkList.tsx
@@ -11,19 +11,17 @@ export default function SdkList() {
     <>
       {sdkList.length !== 0 ? (
         <CardList>
-          {sdkList.map(sdk => {
-            return (
-              <div className="flex items-center gap-x-4 px-6 py-2" key={`${sdk.name}-${sdk.version}`}>
-                <PlatformIcon className="rounded-md" platform={sdkToPlatform(sdk.name)} />
+          {sdkList.map(sdk => (
+            <div className="flex items-center gap-x-4 px-6 py-2" key={`${sdk.name}-${sdk.version}`}>
+              <PlatformIcon className="rounded-md" platform={sdkToPlatform(sdk.name)} />
 
-                <div className="text-primary-300 flex flex-col truncate font-mono text-sm">
-                  <div>{sdk.name}</div>
-                  <div>{sdk.version}</div>
-                  <TimeSince date={sdk.lastSeen} />
-                </div>
+              <div className="text-primary-300 flex flex-col truncate font-mono text-sm">
+                <div>{sdk.name}</div>
+                <div>{sdk.version}</div>
+                <TimeSince date={sdk.lastSeen} />
               </div>
-            );
-          })}
+            </div>
+          ))}
         </CardList>
       ) : (
         <div className="text-primary-300 p-6">Looks like there's no SDKs that have reported yet. 🤔</div>

diff --git a/packages/overlay/src/integrations/sentry/components/SpanResizer.tsx b/packages/overlay/src/integrations/sentry/components/SpanResizer.tsx
@@ -1,4 +1,4 @@
-import { MouseEventHandler, useState } from 'react';
+import { useState, type MouseEventHandler } from 'react';
 import classNames from '~/lib/classNames';
 
 type SpanResizerProps = {

diff --git a/packages/overlay/src/integrations/sentry/components/Tags.tsx b/packages/overlay/src/integrations/sentry/components/Tags.tsx
@@ -3,9 +3,9 @@ import Tag from '~/ui/Tag';
 export default function Tags({ tags }: { tags: { [key: string]: string } }) {
   return (
     <div className="flex flex-row flex-wrap gap-2 pt-2">
-      {Object.keys(tags).map(tagKey => {
-        return <Tag key={tagKey} tagKey={tagKey} value={tags[tagKey]}></Tag>;
-      })}
+      {Object.keys(tags).map(tagKey => (
+        <Tag key={tagKey} tagKey={tagKey} value={tags[tagKey]} />
+      ))}
     </div>
   );
 }
diff --git a/packages/overlay/src/integrations/sentry/components/developerInfo/EnvelopeDetails.tsx b/packages/overlay/src/integrations/sentry/components/developerInfo/EnvelopeDetails.tsx
@@ -1,6 +1,6 @@
-import { Envelope } from '@sentry/types';
+import type { Envelope } from '@sentry/types';
 import { useState } from 'react';
-import { RawEventContext } from '~/integrations/integration';
+import type { RawEventContext } from '~/integrations/integration';
 import SidePanel, { SidePanelHeader } from '~/ui/SidePanel';
 import JsonViewer from './JsonViewer';
 
@@ -14,13 +14,11 @@ export default function EnvelopeDetails({ data }: { data: { envelope: Envelope;
       <SidePanelHeader
         title="Envelope Details"
         subtitle={
-          <>
-            {header.event_id && (
-              <>
-                Event Id <span className="text-primary-500">&mdash;</span> {header.event_id}
-              </>
-            )}
-          </>
+          header.event_id ? (
+            <>
+              Event Id <span className="text-primary-500">&mdash;</span> {header.event_id}
+            </>
+          ) : undefined
         }
         backto="/devInfo"
       />
@@ -33,8 +31,8 @@ export default function EnvelopeDetails({ data }: { data: { envelope: Envelope;
             onChange={() => setShowRawJSON(prev => !prev)}
             checked={showRawJSON}
           />
-          <div className="bg-primary-400 h-4 w-10 rounded-full shadow-inner"></div>
-          <div className="dot absolute -left-1 -top-1 h-6 w-6 rounded-full bg-white shadow transition"></div>
+          <div className="bg-primary-400 h-4 w-10 rounded-full shadow-inner" />
+          <div className="dot absolute -left-1 -top-1 h-6 w-6 rounded-full bg-white shadow transition" />
         </div>
         <span className="ml-2">Show Raw Data</span>
       </label>

diff --git a/packages/overlay/src/integrations/sentry/components/developerInfo/EnvelopeList.tsx b/packages/overlay/src/integrations/sentry/components/developerInfo/EnvelopeList.tsx
@@ -26,7 +26,7 @@ export default function EnvelopeList() {
     ? sentryDataCache.getEnvelopes().find(({ envelope: _env }) => _env[0].event_id === eventId) || null
     : null;
 
-  if (allEnvelopes && allEnvelopes.length) {
+  if (allEnvelopes?.length) {
     return (
       <>
         {hiddenItemCount > 0 && !showAll && (
@@ -47,44 +47,43 @@ export default function EnvelopeList() {
               const envelopeEventId: string | unknown = header.event_id;
               const { trace_id } = (header?.trace as { trace_id?: string }) || {};
               const envelopeItem = envelope[1].length > 0 ? (envelope[1][0] as EnvelopeItem) : null;
-              if (typeof envelopeEventId === 'string') {
-                return (
-                  <Link key={envelopeEventId} to={`/devInfo/${header.event_id}`}>
-                    <div
-                      key={`${header.event_id}`}
-                      className={classNames(
-                        'hover:bg-primary-900 border-b-primary-900 flex cursor-pointer items-center gap-4 border-b px-6 py-2 transition-all',
-                        eventId === envelopeEventId ? 'bg-primary-900' : '',
-                      )}
-                    >
-                      <PlatformIcon className="rounded-md" platform={sdkToPlatform(header.sdk?.name || 'unknown')} />
-                      <div className="text-primary-300 flex flex-[0.25] flex-col truncate font-mono text-sm">
-                        <h2 className="text-primary-50 text-xs">Event Id</h2>
-                        <div className="flex items-center gap-x-2">
-                          <div>{envelopeEventId.substring(0, 8)}</div>
-                          {trace_id && helpers.isLocalToSession(trace_id) ? (
-                            <Badge title="This trace is part of your local session.">Local</Badge>
-                          ) : null}
-                        </div>
+              if (typeof envelopeEventId !== 'string') {
+                return null;
+              }
+              return (
+                <Link key={envelopeEventId} to={`/devInfo/${header.event_id}`}>
+                  <div
+                    className={classNames(
+                      'hover:bg-primary-900 border-b-primary-900 flex cursor-pointer items-center gap-4 border-b px-6 py-2 transition-all',
+                      eventId === envelopeEventId ? 'bg-primary-900' : '',
+                    )}
+                  >
+                    <PlatformIcon className="rounded-md" platform={sdkToPlatform(header.sdk?.name || 'unknown')} />
+                    <div className="text-primary-300 flex flex-[0.25] flex-col truncate font-mono text-sm">
+                      <h2 className="text-primary-50 text-xs">Event Id</h2>
+                      <div className="flex items-center gap-x-2">
+                        <div>{envelopeEventId.substring(0, 8)}</div>
+                        {trace_id && helpers.isLocalToSession(trace_id) ? (
+                          <Badge title="This trace is part of your local session.">Local</Badge>
+                        ) : null}
                       </div>
+                    </div>
 
-                      <div className="text-primary-300 flex flex-[0.25] flex-col truncate font-mono text-sm">
-                        <h2 className="text-primary-50 text-xs">Type</h2>
-                        {envelopeItem && envelopeItem[0]?.type ? <>{envelopeItem[0].type}</> : '-'}
-                      </div>
-                      <div className="text-primary-300 flex flex-[0.25] flex-col truncate font-mono text-sm">
-                        <h2 className="text-primary-50 text-xs">Recieved</h2>
-                        {(header.sent_at as string | Date | number) ? (
-                          <TimeSince date={header.sent_at as string | Date | number} />
-                        ) : (
-                          '-'
-                        )}
-                      </div>
+                    <div className="text-primary-300 flex flex-[0.25] flex-col truncate font-mono text-sm">
+                      <h2 className="text-primary-50 text-xs">Type</h2>
+                      {envelopeItem?.[0]?.type ? envelopeItem[0].type : '-'}
                     </div>
-                  </Link>
-                );
-              }
-              return null;
+                    <div className="text-primary-300 flex flex-[0.25] flex-col truncate font-mono text-sm">
+                      <h2 className="text-primary-50 text-xs">Received</h2>
+                      {(header.sent_at as string | Date | number) ? (
+                        <TimeSince date={header.sent_at as string | Date | number} />
+                      ) : (
+                        '-'
+                      )}
+                    </div>
+                  </div>
+                </Link>
+              );
             })}
           </div>
         </div>

diff --git a/packages/overlay/src/integrations/sentry/components/events/Event.tsx b/packages/overlay/src/integrations/sentry/components/events/Event.tsx
@@ -1,26 +1,24 @@
-import { SentryEvent } from '../../types';
-import { Error, ErrorSummary, ErrorTitle } from './error/Error';
+import type { SentryEvent } from '../../types';
+import { ErrorItem, ErrorSummary, ErrorTitle } from './error/Error';
 
 function getEventMessage(event: SentryEvent) {
   if (typeof event.message === 'string') {
     return event.message;
-  } else if (event.message !== undefined && typeof event.message.formatted === 'string') {
+  }
+
+  if (event.message !== undefined && typeof event.message.formatted === 'string') {
     return event.message.formatted;
-  } else {
-    return '';
   }
+
+  return '';
 }
 
 export function EventTitle({ event }: { event: SentryEvent }) {
   if ('exception' in event) {
     return <ErrorTitle event={event} />;
   }
 
-  return (
-    <>
-      <strong className="font-bold">{getEventMessage(event)}</strong>
-    </>
-  );
+  return <strong className="font-bold">{getEventMessage(event)}</strong>;
 }
 
 export function EventSummary({ event }: { event: SentryEvent }) {
@@ -38,7 +36,7 @@ export function EventSummary({ event }: { event: SentryEvent }) {
 
 export default function Event({ event }: { event: SentryEvent }) {
   if ('exception' in event) {
-    return <Error event={event} />;
+    return <ErrorItem event={event} />;
   }
 
   return (

diff --git a/packages/overlay/src/integrations/sentry/components/events/EventBreadcrumbs.tsx b/packages/overlay/src/integrations/sentry/components/events/EventBreadcrumbs.tsx
@@ -35,10 +35,10 @@ export default function EventBreadcrumbs({ event }: { event: SentryEvent }) {
   }
   return (
     <div className="divide-primary-800 grid-cols-2-auto -mx-2 grid space-y-2 divide-y">
-      {breadcrumbs.map((crumb, crumbIdx) => {
-        if (!crumb.message) return null;
-        return (
-          <Fragment key={crumbIdx}>
+      {breadcrumbs
+        .filter(crumb => crumb.message)
+        .map((crumb, crumbIdx) => (
+          <Fragment key={`${crumb.timestamp}-${crumb.category}-${crumb.type}`}>
             <div className="flex flex-none flex-col p-2">
               <div className="text-lg font-semibold">{crumb.category || ' '}</div>
               <div className="text-primary-300 text-xs">
@@ -54,8 +54,7 @@ export default function EventBreadcrumbs({ event }: { event: SentryEvent }) {
               {crumb.message}
             </pre>
           </Fragment>
-        );
-      })}
+        ))}
     </div>
   );
 }