fix: Don't depend on browser types in types (#9682)

This is a hotfix for #9679. We leaked Browser/React types into the generic `@sentry/types` package which should not have any runtime specific types.
getsentry · Nov 28, 2023 · 98d4988 · 98d4988
1 parent f2e57ad
commit 98d4988
Show file tree

Hide file tree

Showing 7 changed files with 15 additions and 6 deletions.
diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts
@@ -41,7 +41,7 @@ export const handleDomListener: (replay: ReplayContainer) => (handlerData: Handl
       handleClick(
         replay.clickDetector,
         result as Breadcrumb & { timestamp: number; data: { nodeId: number } },
-        getClickTargetNode(handlerData.event) as HTMLElement,
+        getClickTargetNode(handlerData.event as Event) as HTMLElement,
       );
     }
 
@@ -97,7 +97,7 @@ function getDomTarget(handlerData: HandlerDataDom): { target: Node | null; messa
 
   // Accessing event.target can throw (see getsentry/raven-js#838, #768)
   try {
-    target = isClick ? getClickTargetNode(handlerData.event) : getTargetNode(handlerData.event);
+    target = isClick ? getClickTargetNode(handlerData.event as Event) : getTargetNode(handlerData.event as Event);
     message = htmlTreeAsString(target, { maxStringLength: 200 }) || '<unknown>';
   } catch (e) {
     message = '<unknown>';

diff --git a/packages/replay/src/coreHandlers/util/domUtils.ts b/packages/replay/src/coreHandlers/util/domUtils.ts
@@ -1,5 +1,4 @@
 import type { INode } from '@sentry-internal/rrweb-snapshot';
-import type { HandlerDataDom } from '@sentry/types';
 
 const INTERACTIVE_SELECTOR = 'button,a';
 
@@ -15,7 +14,7 @@ export function getClosestInteractive(element: Element): Element {
  * This is useful because if you click on the image in <button><img></button>,
  * The target will be the image, not the button, which we don't want here
  */
-export function getClickTargetNode(event: HandlerDataDom['event'] | MouseEvent | Node): Node | INode | null {
+export function getClickTargetNode(event: Event | MouseEvent | Node): Node | INode | null {
   const target = getTargetNode(event);
 
   if (!target || !(target instanceof Element)) {

diff --git a/packages/types/src/instrument.ts b/packages/types/src/instrument.ts
@@ -63,7 +63,8 @@ export interface HandlerDataFetch {
 }
 
 export interface HandlerDataDom {
-  event: Event | { target: EventTarget };
+  // TODO: Replace `object` here with a vendored type for browser Events. We can't depend on the `DOM` or `react` TS types package here.
+  event: object | { target: object };
   name: string;
   global?: boolean;
 }
@@ -82,7 +83,8 @@ export interface HandlerDataError {
   column?: number;
   error?: Error;
   line?: number;
-  msg: string | Event;
+  // TODO: Replace `object` here with a vendored type for browser Events. We can't depend on the `DOM` or `react` TS types package here.
+  msg: string | object;
   url?: string;
 }
 

diff --git a/packages/utils/src/instrument/dom.ts b/packages/utils/src/instrument/dom.ts
@@ -1,3 +1,5 @@
+// TODO(v8): Move everything in this file into the browser package. Nothing here is generic and we run risk of leaking browser types into non-browser packages.
+
 /* eslint-disable @typescript-eslint/no-explicit-any */
 /* eslint-disable @typescript-eslint/ban-types */
 import type { HandlerDataDom } from '@sentry/types';

diff --git a/packages/utils/src/instrument/history.ts b/packages/utils/src/instrument/history.ts
@@ -1,3 +1,5 @@
+// TODO(v8): Move everything in this file into the browser package. Nothing here is generic and we run risk of leaking browser types into non-browser packages.
+
 /* eslint-disable @typescript-eslint/no-explicit-any */
 /* eslint-disable @typescript-eslint/ban-types */
 import type { HandlerDataHistory } from '@sentry/types';

diff --git a/packages/utils/src/instrument/index.ts b/packages/utils/src/instrument/index.ts
@@ -1,3 +1,5 @@
+// TODO(v8): Consider moving this file (or at least parts of it) into the browser package. The registered handlers are mostly non-generic and we risk leaking runtime specific code into generic packages.
+
 import { DEBUG_BUILD } from '../debug-build';
 import type {
   InstrumentHandlerCallback as _InstrumentHandlerCallback,

diff --git a/packages/utils/src/instrument/xhr.ts b/packages/utils/src/instrument/xhr.ts
@@ -1,3 +1,5 @@
+// TODO(v8): Move everything in this file into the browser package. Nothing here is generic and we run risk of leaking browser types into non-browser packages.
+
 /* eslint-disable @typescript-eslint/no-explicit-any */
 /* eslint-disable @typescript-eslint/ban-types */
 import type { HandlerDataXhr, SentryWrappedXMLHttpRequest, WrappedFunction } from '@sentry/types';