Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core)!: Remove getDomElement method #14797

Merged
merged 3 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/migration/v8-to-v9.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ It will be removed in a future major version.
- The `BAGGAGE_HEADER_NAME` export has been removed. Use `"baggage"` string constant directly instead.
- The `flatten` export has been removed. There is no replacement.
- The `urlEncode` method has been removed. There is no replacement.
- The `getDomElement` method has been removed. There is no replacement.

### `@sentry/nestjs`

Expand Down
23 changes: 12 additions & 11 deletions packages/browser/src/tracing/browserTracingIntegration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
getActiveSpan,
getClient,
getCurrentScope,
getDomElement,
getDynamicSamplingContextFromSpan,
getIsolationScope,
getRootSpan,
Expand All @@ -39,6 +38,12 @@ import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from

export const BROWSER_TRACING_INTEGRATION_ID = 'BrowserTracing';

/**
* This is just a small wrapper that makes `document` optional.
* We want to be extra-safe and always check that this exists, to ensure weird environments do not blow up.
*/
const optionalWindowDocument = WINDOW.document as (typeof WINDOW)['document'] | undefined;

interface RouteInfo {
name: string | undefined;
source: TransactionSource | undefined;
Expand Down Expand Up @@ -273,13 +278,13 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
});

function emitFinish(): void {
if (['interactive', 'complete'].includes(WINDOW.document.readyState)) {
if (optionalWindowDocument && ['interactive', 'complete'].includes(optionalWindowDocument.readyState)) {
client.emit('idleSpanEnableAutoFinish', idleSpan);
}
}

if (isPageloadTransaction && WINDOW.document) {
WINDOW.document.addEventListener('readystatechange', () => {
if (isPageloadTransaction && optionalWindowDocument) {
optionalWindowDocument.addEventListener('readystatechange', () => {
emitFinish();
});

Expand Down Expand Up @@ -462,12 +467,8 @@ export function startBrowserTracingNavigationSpan(client: Client, spanOptions: S

/** Returns the value of a meta tag */
export function getMetaContent(metaName: string): string | undefined {
// Can't specify generic to `getDomElement` because tracing can be used
// in a variety of environments, have to disable `no-unsafe-member-access`
// as a result.
const metaTag = getDomElement(`meta[name=${metaName}]`);
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
return metaTag ? metaTag.getAttribute('content') : undefined;
const metaTag = optionalWindowDocument && optionalWindowDocument.querySelector(`meta[name=${metaName}]`);
return (metaTag && metaTag.getAttribute('content')) || undefined;
}

/** Start listener for interaction transactions */
Expand Down Expand Up @@ -519,7 +520,7 @@ function registerInteractionListener(
);
};

if (WINDOW.document) {
if (optionalWindowDocument) {
addEventListener('click', registerInteractionTransaction, { once: false, capture: true });
}
}
24 changes: 0 additions & 24 deletions packages/core/src/utils-hoist/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,30 +140,6 @@ export function getLocationHref(): string {
}
}

/**
* Gets a DOM element by using document.querySelector.
*
* This wrapper will first check for the existence of the function before
* actually calling it so that we don't have to take care of this check,
* every time we want to access the DOM.
*
* Reason: DOM/querySelector is not available in all environments.
*
* We have to cast to any because utils can be consumed by a variety of environments,
* and we don't want to break TS users. If you know what element will be selected by
* `document.querySelector`, specify it as part of the generic call. For example,
* `const element = getDomElement<Element>('selector');`
*
* @param selector the selector string passed on to document.querySelector
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function getDomElement<E = any>(selector: string): E | null {
if (WINDOW.document && WINDOW.document.querySelector) {
return WINDOW.document.querySelector(selector) as unknown as E;
}
return null;
}

/**
* Given a DOM element, traverses up the tree until it finds the first ancestor node
* that has the `data-sentry-component` or `data-sentry-element` attribute with `data-sentry-component` taking
Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/utils-hoist/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
export { applyAggregateErrorsToEvent } from './aggregate-errors';
export { getBreadcrumbLogLevelFromHttpStatusCode } from './breadcrumb-log-level';
export { getComponentName, getDomElement, getLocationHref, htmlTreeAsString } from './browser';
export {
getComponentName,
getLocationHref,
htmlTreeAsString,
} from './browser';
export { dsnFromString, dsnToString, makeDsn } from './dsn';
export { SentryError } from './error';
export { GLOBAL_OBJ } from './worldwide';
Expand Down
12 changes: 1 addition & 11 deletions packages/core/test/utils-hoist/browser.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { JSDOM } from 'jsdom';

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

beforeAll(() => {
const dom = new JSDOM();
Expand Down Expand Up @@ -74,13 +74,3 @@ describe('htmlTreeAsString', () => {
);
});
});

describe('getDomElement', () => {
it('returns the element for a given query selector', () => {
document.head.innerHTML = '<div id="mydiv">Hello</div>';
const el = getDomElement('div#mydiv');
expect(el).toBeDefined();
expect(el?.tagName).toEqual('DIV');
expect(el?.id).toEqual('mydiv');
});
});
5 changes: 3 additions & 2 deletions packages/svelte/src/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import type { BrowserOptions } from '@sentry/browser';
import { WINDOW } from '@sentry/browser';
import { addEventProcessor, init as browserInit } from '@sentry/browser';
import type { Client, EventProcessor } from '@sentry/core';
import { applySdkMetadata, getDomElement } from '@sentry/core';
import { applySdkMetadata } from '@sentry/core';
/**
* Inits the Svelte SDK
*/
Expand Down Expand Up @@ -55,5 +56,5 @@ export function detectAndReportSvelteKit(): void {
* @see https://github.com/sveltejs/kit/issues/307 for more information
*/
export function isSvelteKitApp(): boolean {
return getDomElement('div#svelte-announcer') !== null;
return !!WINDOW.document.querySelector('div#svelte-announcer');
}
4 changes: 0 additions & 4 deletions packages/utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ import {
getBreadcrumbLogLevelFromHttpStatusCode as getBreadcrumbLogLevelFromHttpStatusCode_imported,
getComponentName as getComponentName_imported,
getDebugImagesForResources as getDebugImagesForResources_imported,
getDomElement as getDomElement_imported,
getEventDescription as getEventDescription_imported,
getFilenameToDebugIdMap as getFilenameToDebugIdMap_imported,
getFramesFromEvent as getFramesFromEvent_imported,
Expand Down Expand Up @@ -542,9 +541,6 @@ export const resolve = resolve_imported;
/** @deprecated Import from `@sentry/core` instead. */
export const getComponentName = getComponentName_imported;

/** @deprecated Import from `@sentry/core` instead. */
export const getDomElement = getDomElement_imported;

/** @deprecated Import from `@sentry/core` instead. */
export const getLocationHref = getLocationHref_imported;

Expand Down
Loading