Skip to content

Commit

Permalink
fix: Various React improvements (#476)
Browse files Browse the repository at this point in the history
  • Loading branch information
BYK committed Aug 6, 2024
1 parent d38c054 commit 8f42d4e
Show file tree
Hide file tree
Showing 16 changed files with 155 additions and 133 deletions.
8 changes: 8 additions & 0 deletions .changeset/twenty-ghosts-float.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@spotlightjs/overlay': patch
'@spotlightjs/astro': patch
'@spotlightjs/electron': patch
'@spotlightjs/spotlight': patch
---

Various improvements in React code for better stability & performance
1 change: 1 addition & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"Astro",
"Astro's",
"contextlines",
"Endcaps",
"fontsource",
"pageload",
"spotlightjs",
Expand Down
137 changes: 76 additions & 61 deletions packages/overlay/src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useState } from 'react';
import { useEffect, useMemo, useState } from 'react';
import { useNavigate } from 'react-router-dom';
import Debugger from './components/Debugger';
import Trigger from './components/Trigger';
Expand All @@ -7,7 +7,7 @@ import { getSpotlightEventTarget } from './lib/eventTarget';
import { log } from './lib/logger';
import useKeyPress from './lib/useKeyPress';
import { connectToSidecar } from './sidecar';
import { NotificationCount, SpotlightOverlayOptions } from './types';
import type { NotificationCount, SpotlightOverlayOptions } from './types';

type AppProps = Omit<SpotlightOverlayOptions, 'debug' | 'injectImmediately'> &
Required<Pick<SpotlightOverlayOptions, 'sidecarUrl'>>;
Expand All @@ -21,65 +21,69 @@ export default function App({
fullPage = false,
showClearEventsButton = true,
}: AppProps) {
log('App rerender');
const [integrationData, setIntegrationData] = useState<IntegrationData<unknown>>({});
const [isOnline, setOnline] = useState(false);
const [triggerButtonCount, setTriggerButtonCount] = useState<NotificationCount>({ count: 0, severe: false });
const [isOpen, setOpen] = useState(openOnInit);
const [reloadSpotlight, setReloadSpotlight] = useState<number>(0);

useKeyPress(['ctrlKey', 'F12'], () => {
setOpen(prev => !prev);
});
log('App rerender', integrationData, isOnline, triggerButtonCount, isOpen);

// Map that holds the information which kind of content type should be dispatched to which integration(s)
const contentTypeToIntegrations = useMemo(() => {
const result = new Map<string, Integration[]>();
for (const integration of integrations) {
if (!integration.forwardedContentType) continue;

for (const contentType of integration.forwardedContentType) {
let integrationsForContentType = result.get(contentType);
if (!integrationsForContentType) {
integrationsForContentType = [];
result.set(contentType, integrationsForContentType);
}
integrationsForContentType.push(integration);
}
}
return result;
}, [integrations]);

useEffect(() => {
// Map that holds the information which kind of content type should be dispatched to which integration(s)
const contentTypeToIntegrations = new Map<string, Integration[]>();

integrations.forEach(
integration =>
integration.forwardedContentType?.forEach(contentType => {
const i = contentTypeToIntegrations.get(contentType) || [];
i.push(integration);
contentTypeToIntegrations.set(contentType, i);
}),
);

const cleanupListeners = connectToSidecar(sidecarUrl, contentTypeToIntegrations, setIntegrationData, setOnline);

return () => {
log('useEffect cleanup');
cleanupListeners();
};
}, [integrations, sidecarUrl, reloadSpotlight]);
useEffect(
() => connectToSidecar(sidecarUrl, contentTypeToIntegrations, setIntegrationData, setOnline) as () => undefined,
[sidecarUrl, contentTypeToIntegrations],
);

const spotlightEventTarget = getSpotlightEventTarget();
const spotlightEventTarget = useMemo(() => getSpotlightEventTarget(), []);

// Note that `useNavigate()` relies on `useLocation()` which
// causes a full re-render here every time we change the location
// We can fix this by doing `const router = createMemoryRouter()`
// and using `router.navigate()` but that requires a larger refactor
// as our <Route>s are scattered around a bit
// See https://github.com/remix-run/react-router/issues/7634
const navigate = useNavigate();
const eventHandlers = useMemo(() => {
log('useMemo: initializing event handlers');
const clearEvents = async () => {
const { origin } = new URL(sidecarUrl);
const clearEventsUrl: string = `${origin}/clear`;

try {
await fetch(clearEventsUrl, {
method: 'DELETE',
mode: 'cors',
});
} catch (err) {
console.error(
`Spotlight can't connect to Sidecar is it running? See: https://spotlightjs.com/sidecar/npx/`,
err,
);
return;
}

const clearEvents = () => {
const { origin } = new URL(sidecarUrl);
const clearEventsUrl: string = `${origin}/clear`;

fetch(clearEventsUrl, {
method: 'DELETE',
mode: 'cors',
}).catch(err => {
console.error(`Spotlight can't connect to Sidecar is it running? See: https://spotlightjs.com/sidecar/npx/`, err);
return;
});

integrations.forEach(integration => {
const IntegrationName = integration.name;
if (integration.processEvent) {
setIntegrationData(prev => ({ ...prev, [IntegrationName]: [] }));
for (const integration of integrations) {
setIntegrationData(prev => ({ ...prev, [integration.name]: [] }));
if (integration.reset) integration.reset();
}
});
setReloadSpotlight(prev => prev + 1);
};
};

useEffect(() => {
const onOpen = (
e: CustomEvent<{
path: string | undefined;
Expand All @@ -95,23 +99,36 @@ export default function App({
setOpen(false);
};

const onToggle = () => {
log('Toggle');
setOpen(prev => !prev);
};

const onNavigate = (e: CustomEvent<string>) => {
log('Navigate');
navigate(e.detail);
};

spotlightEventTarget.addEventListener('open', onOpen as EventListener);
spotlightEventTarget.addEventListener('close', onClose);
spotlightEventTarget.addEventListener('navigate', onNavigate as EventListener);
spotlightEventTarget.addEventListener('clearEvents', clearEvents as EventListener);
return { clearEvents, onOpen, onClose, onNavigate, onToggle };
}, [integrations, navigate, sidecarUrl]);

return () => {
spotlightEventTarget.removeEventListener('open', onOpen as EventListener);
spotlightEventTarget.removeEventListener('close', onClose);
spotlightEventTarget.removeEventListener('navigate', onNavigate as EventListener);
spotlightEventTarget.removeEventListener('clearEvents', clearEvents as EventListener);
useKeyPress(['ctrlKey', 'F12'], eventHandlers.onToggle);

useEffect(() => {
log('useEffect: Adding event listeners');
spotlightEventTarget.addEventListener('open', eventHandlers.onOpen as EventListener);
spotlightEventTarget.addEventListener('close', eventHandlers.onClose);
spotlightEventTarget.addEventListener('navigate', eventHandlers.onNavigate as EventListener);
spotlightEventTarget.addEventListener('clearEvents', eventHandlers.clearEvents as EventListener);

return (): undefined => {
log('useEffect[destructor]: Removing event listeners');
spotlightEventTarget.removeEventListener('open', eventHandlers.onOpen as EventListener);
spotlightEventTarget.removeEventListener('close', eventHandlers.onClose);
spotlightEventTarget.removeEventListener('navigate', eventHandlers.onNavigate as EventListener);
spotlightEventTarget.removeEventListener('clearEvents', eventHandlers.clearEvents as EventListener);
};
}, [spotlightEventTarget, navigate]);
}, [spotlightEventTarget, eventHandlers]);

useEffect(() => {
if (!isOpen) {
Expand All @@ -131,8 +148,6 @@ export default function App({
}
}, [triggerButtonCount, spotlightEventTarget]);

log('Integration data:', integrationData);

return (
<>
{showTriggerButton && (
Expand Down
4 changes: 2 additions & 2 deletions packages/overlay/src/components/Debugger.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { ReactComponent as DeleteIcon } from '~/assets/deleteIcon.svg';
import { ReactComponent as Logo } from '~/assets/glyph.svg';
import { Integration, IntegrationData } from '~/integrations/integration';
import type { Integration, IntegrationData } from '~/integrations/integration';
import { getSpotlightEventTarget } from '~/lib/eventTarget';
import { NotificationCount } from '~/types';
import type { NotificationCount } from '~/types';
import classNames from '../lib/classNames';
import Overview from './Overview';

Expand Down
26 changes: 12 additions & 14 deletions packages/overlay/src/components/Overview.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { createElement, useEffect, useState } from 'react';
import { Route, Routes } from 'react-router-dom';
import { Integration, IntegrationData } from '~/integrations/integration';
import { NotificationCount } from '~/types';
import type { Integration, IntegrationData } from '~/integrations/integration';
import type { NotificationCount } from '~/types';
import Tabs from './Tabs';

export default function Overview({
Expand All @@ -17,18 +17,16 @@ export default function Overview({
}) {
const [notificationCountSum, setNotificationCountSum] = useState<NotificationCount>({ count: 0, severe: false });

const tabs = integrations
.map(integration => {
if (integration.tabs) {
const processedEvents = integrationData[integration.name]?.map(container => container.event) || [];
return integration.tabs({ processedEvents }).map(tab => ({
...tab,
processedEvents: processedEvents,
}));
}
return [];
})
.flat();
const tabs = integrations.flatMap(integration => {
if (integration.tabs) {
const processedEvents = integrationData[integration.name]?.map(container => container.event) || [];
return integration.tabs({ processedEvents }).map(tab => ({
...tab,
processedEvents: processedEvents,
}));
}
return [];
});

const newNotificationSum = tabs.reduce(
(sum, tab) => ({
Expand Down
24 changes: 10 additions & 14 deletions packages/overlay/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { off, on, trigger } from './lib/eventTarget.ts';
import { activateLogger, log } from './lib/logger.ts';
import { SpotlightContextProvider } from './lib/useSpotlightContext.tsx';
import { React, ReactDOM } from './react-instance.tsx'; // Import specific exports
import { SpotlightOverlayOptions, WindowWithSpotlight } from './types.ts';
import type { SpotlightOverlayOptions, WindowWithSpotlight } from './types.ts';

export { default as console } from './integrations/console/index.ts';
export { default as hydrationError } from './integrations/hydration-error/index.ts';
Expand All @@ -23,10 +23,10 @@ export {
DEFAULT_ANCHOR,
DEFAULT_EXPERIMENTS,
DEFAULT_SIDECAR_URL,
React,
ReactDOM,
off,
on,
React,
ReactDOM,
trigger,
};

Expand Down Expand Up @@ -146,17 +146,13 @@ export async function init({
});
}

const tabs = initializedIntegrations
.map(integration => {
if (integration.tabs) {
return integration.tabs({ processedEvents: [] }).map(tab => ({
...tab,
processedEvents: [],
}));
}
return [];
})
.flat();
const tabs = initializedIntegrations.flatMap(
integration =>
integration.tabs?.({ processedEvents: [] }).map(tab => ({
...tab,
processedEvents: [],
})) || [],
);

const initialTab = tabs.length ? `/${tabs[0].id}` : '/no-tabs';

Expand Down
14 changes: 6 additions & 8 deletions packages/overlay/src/integrations/console/console-tab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,12 @@ export default function ConsoleTab({ processedEvents }: Props) {
<div className="divide-primary-900 bg-primary-950 divide-y p-4">
<h1 className="mb-2 text-xl">Console Logs</h1>
<div className="flex flex-col gap-2">
{messages.map((message, index) => {
return (
<div key={index} className="bg-primary-500 py-4 pl-4">
<span className="bg-primary-600 p-2 font-bold">{message.type.toUpperCase()}</span>
<span className="ml-4 font-mono">{message.msg}</span>
</div>
);
})}
{messages.map(message => (
<div key={message.msg} className="bg-primary-500 py-4 pl-4">
<span className="bg-primary-600 p-2 font-bold">{message.type.toUpperCase()}</span>
<span className="ml-4 font-mono">{message.msg}</span>
</div>
))}
</div>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export default function HiddenItemsButton({
<strong>
{itemCount.toLocaleString()} {itemCount !== 1 ? 'items were' : 'item was'} hidden from different sessions.
</strong>
<a className="hover:bg-primary-900 border-primary-500 rounded border px-1.5 py-0.5">Reveal</a>
<span className="hover:bg-primary-900 border-primary-500 rounded border px-1.5 py-0.5">Reveal</span>
</button>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useState } from 'react';
import { Link, Outlet, Route, Routes, useParams } from 'react-router-dom';
import Tabs from '../../../../components/Tabs';
import sentryDataCache from '../../data/sentryDataCache';
import { SentryEvent } from '../../types';
import type { SentryEvent } from '../../types';
import PlatformIcon from '../PlatformIcon';
import Event, { EventTitle } from './Event';
import EventBreadcrumbs from './EventBreadcrumbs';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ const PerformanceChart = ({
() => setWebVitalTooltip('cls'),
() => setWebVitalTooltip('ttfb'),
]}
onUnhover={() => setWebVitalTooltip(null)}
onBlur={() => setWebVitalTooltip(null)}
/>
</svg>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Trace } from '../../types';
import type { Trace } from '../../types';
import { sdkToPlatform } from '../../utils/sdkToPlatform';
import PlatformIcon from '../PlatformIcon';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@ export const SentryEventsContext = React.createContext<SentryEventsContextProps>
});

function eventReducer(state: SentryEvent[], message: SetEventsAction): SentryEvent[] {
if (message.action === 'RESET' && Array.isArray(message.e)) {
return message.e;
} else if (message.action === 'APPEND' && !Array.isArray(message.e)) {
return [message.e, ...state];
if (Array.isArray(message.e)) {
if (message.action === 'RESET') {
return message.e;
}
} else {
if (message.action === 'APPEND') {
return [message.e, ...state];
}
}

return state;
Expand All @@ -33,11 +37,13 @@ export const SentryEventsContextProvider: React.FC<{
}> = ({ children }) => {
const [events, setEvents] = useReducer(eventReducer, sentryDataCache.getEvents());

useEffect(() => {
return sentryDataCache.subscribe('event', (e: SentryEvent) => {
setEvents({ action: 'APPEND', e });
});
}, []);
useEffect(
() =>
sentryDataCache.subscribe('event', (e: SentryEvent) => {
setEvents({ action: 'APPEND', e });
}) as () => undefined,
[],
);

const contextValue: SentryEventsContextProps = {
events,
Expand Down
2 changes: 1 addition & 1 deletion packages/overlay/src/lib/eventTarget.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { WindowWithSpotlight } from '~/types';
import type { WindowWithSpotlight } from '~/types';

const fallbackEventTarget = new EventTarget();

Expand Down
Loading

0 comments on commit 8f42d4e

Please sign in to comment.