Skip to content

Commit 87dcdef

Browse files
authored
feat(core)!: Remove getDomElement method (#14797)
I noticed this in #14758, this seems unnecessary and all browsers/envs we support should be able to handle this. We only use this in two places anyhow, so we can just guard there (a bit) for this instead. I will add a deprecation to v8 for this too, although this is just exported from core and will most likely not affect anybody.
1 parent bc816e9 commit 87dcdef

File tree

7 files changed

+34
-53
lines changed

7 files changed

+34
-53
lines changed

docs/migration/v8-to-v9.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ It will be removed in a future major version.
111111
- The `BAGGAGE_HEADER_NAME` export has been removed. Use `"baggage"` string constant directly instead.
112112
- The `flatten` export has been removed. There is no replacement.
113113
- The `urlEncode` method has been removed. There is no replacement.
114+
- The `getDomElement` method has been removed. There is no replacement.
114115

115116
### `@sentry/nestjs`
116117

packages/browser/src/tracing/browserTracingIntegration.ts

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import {
2121
getActiveSpan,
2222
getClient,
2323
getCurrentScope,
24-
getDomElement,
2524
getDynamicSamplingContextFromSpan,
2625
getIsolationScope,
2726
getRootSpan,
@@ -190,6 +189,12 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = {
190189
* We explicitly export the proper type here, as this has to be extended in some cases.
191190
*/
192191
export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptions> = {}) => {
192+
/**
193+
* This is just a small wrapper that makes `document` optional.
194+
* We want to be extra-safe and always check that this exists, to ensure weird environments do not blow up.
195+
*/
196+
const optionalWindowDocument = WINDOW.document as (typeof WINDOW)['document'] | undefined;
197+
193198
registerSpanErrorInstrumentation();
194199

195200
const {
@@ -273,13 +278,13 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
273278
});
274279

275280
function emitFinish(): void {
276-
if (['interactive', 'complete'].includes(WINDOW.document.readyState)) {
281+
if (optionalWindowDocument && ['interactive', 'complete'].includes(optionalWindowDocument.readyState)) {
277282
client.emit('idleSpanEnableAutoFinish', idleSpan);
278283
}
279284
}
280285

281-
if (isPageloadTransaction && WINDOW.document) {
282-
WINDOW.document.addEventListener('readystatechange', () => {
286+
if (isPageloadTransaction && optionalWindowDocument) {
287+
optionalWindowDocument.addEventListener('readystatechange', () => {
283288
emitFinish();
284289
});
285290

@@ -462,12 +467,14 @@ export function startBrowserTracingNavigationSpan(client: Client, spanOptions: S
462467

463468
/** Returns the value of a meta tag */
464469
export function getMetaContent(metaName: string): string | undefined {
465-
// Can't specify generic to `getDomElement` because tracing can be used
466-
// in a variety of environments, have to disable `no-unsafe-member-access`
467-
// as a result.
468-
const metaTag = getDomElement(`meta[name=${metaName}]`);
469-
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
470-
return metaTag ? metaTag.getAttribute('content') : undefined;
470+
/**
471+
* This is just a small wrapper that makes `document` optional.
472+
* We want to be extra-safe and always check that this exists, to ensure weird environments do not blow up.
473+
*/
474+
const optionalWindowDocument = WINDOW.document as (typeof WINDOW)['document'] | undefined;
475+
476+
const metaTag = optionalWindowDocument && optionalWindowDocument.querySelector(`meta[name=${metaName}]`);
477+
return (metaTag && metaTag.getAttribute('content')) || undefined;
471478
}
472479

473480
/** Start listener for interaction transactions */
@@ -477,6 +484,12 @@ function registerInteractionListener(
477484
childSpanTimeout: BrowserTracingOptions['childSpanTimeout'],
478485
latestRoute: RouteInfo,
479486
): void {
487+
/**
488+
* This is just a small wrapper that makes `document` optional.
489+
* We want to be extra-safe and always check that this exists, to ensure weird environments do not blow up.
490+
*/
491+
const optionalWindowDocument = WINDOW.document as (typeof WINDOW)['document'] | undefined;
492+
480493
let inflightInteractionSpan: Span | undefined;
481494
const registerInteractionTransaction = (): void => {
482495
const op = 'ui.action.click';
@@ -519,7 +532,7 @@ function registerInteractionListener(
519532
);
520533
};
521534

522-
if (WINDOW.document) {
535+
if (optionalWindowDocument) {
523536
addEventListener('click', registerInteractionTransaction, { once: false, capture: true });
524537
}
525538
}

packages/core/src/utils-hoist/browser.ts

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -140,30 +140,6 @@ export function getLocationHref(): string {
140140
}
141141
}
142142

143-
/**
144-
* Gets a DOM element by using document.querySelector.
145-
*
146-
* This wrapper will first check for the existence of the function before
147-
* actually calling it so that we don't have to take care of this check,
148-
* every time we want to access the DOM.
149-
*
150-
* Reason: DOM/querySelector is not available in all environments.
151-
*
152-
* We have to cast to any because utils can be consumed by a variety of environments,
153-
* and we don't want to break TS users. If you know what element will be selected by
154-
* `document.querySelector`, specify it as part of the generic call. For example,
155-
* `const element = getDomElement<Element>('selector');`
156-
*
157-
* @param selector the selector string passed on to document.querySelector
158-
*/
159-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
160-
export function getDomElement<E = any>(selector: string): E | null {
161-
if (WINDOW.document && WINDOW.document.querySelector) {
162-
return WINDOW.document.querySelector(selector) as unknown as E;
163-
}
164-
return null;
165-
}
166-
167143
/**
168144
* Given a DOM element, traverses up the tree until it finds the first ancestor node
169145
* that has the `data-sentry-component` or `data-sentry-element` attribute with `data-sentry-component` taking

packages/core/src/utils-hoist/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
export { applyAggregateErrorsToEvent } from './aggregate-errors';
22
export { getBreadcrumbLogLevelFromHttpStatusCode } from './breadcrumb-log-level';
3-
export { getComponentName, getDomElement, getLocationHref, htmlTreeAsString } from './browser';
3+
export {
4+
getComponentName,
5+
getLocationHref,
6+
htmlTreeAsString,
7+
} from './browser';
48
export { dsnFromString, dsnToString, makeDsn } from './dsn';
59
export { SentryError } from './error';
610
export { GLOBAL_OBJ } from './worldwide';

packages/core/test/utils-hoist/browser.test.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { JSDOM } from 'jsdom';
22

3-
import { getDomElement, htmlTreeAsString } from '../../src/utils-hoist/browser';
3+
import { htmlTreeAsString } from '../../src/utils-hoist/browser';
44

55
beforeAll(() => {
66
const dom = new JSDOM();
@@ -74,13 +74,3 @@ describe('htmlTreeAsString', () => {
7474
);
7575
});
7676
});
77-
78-
describe('getDomElement', () => {
79-
it('returns the element for a given query selector', () => {
80-
document.head.innerHTML = '<div id="mydiv">Hello</div>';
81-
const el = getDomElement('div#mydiv');
82-
expect(el).toBeDefined();
83-
expect(el?.tagName).toEqual('DIV');
84-
expect(el?.id).toEqual('mydiv');
85-
});
86-
});

packages/svelte/src/sdk.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import type { BrowserOptions } from '@sentry/browser';
2+
import { WINDOW } from '@sentry/browser';
23
import { addEventProcessor, init as browserInit } from '@sentry/browser';
34
import type { Client, EventProcessor } from '@sentry/core';
4-
import { applySdkMetadata, getDomElement } from '@sentry/core';
5+
import { applySdkMetadata } from '@sentry/core';
56
/**
67
* Inits the Svelte SDK
78
*/
@@ -55,5 +56,5 @@ export function detectAndReportSvelteKit(): void {
5556
* @see https://github.com/sveltejs/kit/issues/307 for more information
5657
*/
5758
export function isSvelteKitApp(): boolean {
58-
return getDomElement('div#svelte-announcer') !== null;
59+
return !!WINDOW.document.querySelector('div#svelte-announcer');
5960
}

packages/utils/src/index.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ import {
6767
getBreadcrumbLogLevelFromHttpStatusCode as getBreadcrumbLogLevelFromHttpStatusCode_imported,
6868
getComponentName as getComponentName_imported,
6969
getDebugImagesForResources as getDebugImagesForResources_imported,
70-
getDomElement as getDomElement_imported,
7170
getEventDescription as getEventDescription_imported,
7271
getFilenameToDebugIdMap as getFilenameToDebugIdMap_imported,
7372
getFramesFromEvent as getFramesFromEvent_imported,
@@ -542,9 +541,6 @@ export const resolve = resolve_imported;
542541
/** @deprecated Import from `@sentry/core` instead. */
543542
export const getComponentName = getComponentName_imported;
544543

545-
/** @deprecated Import from `@sentry/core` instead. */
546-
export const getDomElement = getDomElement_imported;
547-
548544
/** @deprecated Import from `@sentry/core` instead. */
549545
export const getLocationHref = getLocationHref_imported;
550546

0 commit comments

Comments
 (0)