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

ref(utils): Remove getGlobalObject() usage from all framework specific SDKs #5948

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
8 changes: 3 additions & 5 deletions packages/angular/src/tracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { AfterViewInit, Directive, Injectable, Input, NgModule, OnDestroy, OnIni
import { ActivatedRouteSnapshot, Event, NavigationEnd, NavigationStart, ResolveEnd, Router } from '@angular/router';
import { getCurrentHub } from '@sentry/browser';
import { Span, Transaction, TransactionContext } from '@sentry/types';
import { getGlobalObject, logger, stripUrlQueryAndFragment, timestampWithMs } from '@sentry/utils';
import { logger, stripUrlQueryAndFragment, timestampWithMs, WINDOW } from '@sentry/utils';
import { Observable, Subscription } from 'rxjs';
import { filter, tap } from 'rxjs/operators';

Expand All @@ -15,8 +15,6 @@ let instrumentationInitialized: boolean;
let stashedStartTransaction: (context: TransactionContext) => Transaction | undefined;
let stashedStartTransactionOnLocationChange: boolean;

const global = getGlobalObject<Window>();

/**
* Creates routing instrumentation for Angular Router.
*/
Expand All @@ -29,9 +27,9 @@ export function routingInstrumentation(
stashedStartTransaction = customStartTransaction;
stashedStartTransactionOnLocationChange = startTransactionOnLocationChange;

if (startTransactionOnPageLoad && global && global.location) {
if (startTransactionOnPageLoad && WINDOW && WINDOW.location) {
customStartTransaction({
name: global.location.pathname,
name: WINDOW.location.pathname,
op: 'pageload',
metadata: { source: 'url' },
});
Expand Down
5 changes: 2 additions & 3 deletions packages/ember/addon/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@ import { macroCondition, isDevelopingApp, getOwnConfig } from '@embroider/macros
import { next } from '@ember/runloop';
import { assert, warn } from '@ember/debug';
import Ember from 'ember';
import { timestampWithMs } from '@sentry/utils';
import { timestampWithMs, GLOBAL_OBJ } from '@sentry/utils';
import { GlobalConfig, OwnConfig } from './types';
import { getGlobalObject } from '@sentry/utils';

function _getSentryInitConfig() {
const _global = getGlobalObject<GlobalConfig>();
const _global = GLOBAL_OBJ as typeof GLOBAL_OBJ & GlobalConfig;
_global.__sentryEmberConfig = _global.__sentryEmberConfig ?? {};
return _global.__sentryEmberConfig;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ import { ExtendedBackburner } from '@sentry/ember/runloop';
import { Span, Transaction, Integration } from '@sentry/types';
import { EmberRunQueues } from '@ember/runloop/-private/types';
import { getActiveTransaction } from '..';
import { browserPerformanceTimeOrigin, getGlobalObject, timestampWithMs } from '@sentry/utils';
import { browserPerformanceTimeOrigin, GLOBAL_OBJ, timestampWithMs } from '@sentry/utils';
import { macroCondition, isTesting, getOwnConfig } from '@embroider/macros';
import { EmberSentryConfig, GlobalConfig, OwnConfig } from '../types';

function getSentryConfig() {
const _global = getGlobalObject<GlobalConfig>();
const _global = GLOBAL_OBJ as typeof GLOBAL_OBJ & GlobalConfig;
_global.__sentryEmberConfig = _global.__sentryEmberConfig ?? {};
const environmentConfig = getOwnConfig<OwnConfig>().sentryConfig;
if (!environmentConfig.sentry) {
Expand Down
14 changes: 6 additions & 8 deletions packages/nextjs/src/performance/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,19 @@ import { Primitive, TraceparentData, Transaction, TransactionContext, Transactio
import {
baggageHeaderToDynamicSamplingContext,
extractTraceparentData,
getGlobalObject,
logger,
stripUrlQueryAndFragment,
WINDOW,
} from '@sentry/utils';
import type { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils';
import { default as Router } from 'next/router';
import type { ParsedUrlQuery } from 'querystring';

const global = getGlobalObject<
Window & {
__BUILD_MANIFEST?: {
sortedPages?: string[];
};
}
>();
const global = WINDOW as typeof WINDOW & {
__BUILD_MANIFEST?: {
sortedPages?: string[];
};
};

type StartTransactionCb = (context: TransactionContext) => Transaction | undefined;

Expand Down
14 changes: 6 additions & 8 deletions packages/nextjs/test/index.client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,14 @@ import { BaseClient, getCurrentHub } from '@sentry/core';
import * as SentryReact from '@sentry/react';
import { Integrations as TracingIntegrations } from '@sentry/tracing';
import { Integration } from '@sentry/types';
import { getGlobalObject, logger } from '@sentry/utils';
import { logger, WINDOW } from '@sentry/utils';
import { JSDOM } from 'jsdom';

import { init, Integrations, nextRouterInstrumentation } from '../src/index.client';
import { UserIntegrationsFunction } from '../src/utils/userIntegrations';

const { BrowserTracing } = TracingIntegrations;

const global = getGlobalObject();

const reactInit = jest.spyOn(SentryReact, 'init');
const captureEvent = jest.spyOn(BaseClient.prototype, 'captureEvent');
const loggerLogSpy = jest.spyOn(logger, 'log');
Expand All @@ -23,12 +21,12 @@ const dom = new JSDOM(undefined, { url: 'https://example.com/' });
Object.defineProperty(global, 'document', { value: dom.window.document, writable: true });
Object.defineProperty(global, 'location', { value: dom.window.document.location, writable: true });

const originalGlobalDocument = getGlobalObject<Window>().document;
const originalGlobalLocation = getGlobalObject<Window>().location;
const originalGlobalDocument = WINDOW.document;
const originalGlobalLocation = WINDOW.location;
afterAll(() => {
// Clean up JSDom
Object.defineProperty(global, 'document', { value: originalGlobalDocument });
Object.defineProperty(global, 'location', { value: originalGlobalLocation });
Object.defineProperty(WINDOW, 'document', { value: originalGlobalDocument });
Object.defineProperty(WINDOW, 'location', { value: originalGlobalLocation });
});

function findIntegrationByName(integrations: Integration[] = [], name: string): Integration | undefined {
Expand All @@ -38,7 +36,7 @@ function findIntegrationByName(integrations: Integration[] = [], name: string):
describe('Client init()', () => {
afterEach(() => {
jest.clearAllMocks();
global.__SENTRY__.hub = undefined;
WINDOW.__SENTRY__.hub = undefined;
});

it('inits the React SDK', () => {
Expand Down
8 changes: 3 additions & 5 deletions packages/nextjs/test/index.server.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import * as SentryNode from '@sentry/node';
import { getCurrentHub, NodeClient } from '@sentry/node';
import { Integration } from '@sentry/types';
import { getGlobalObject, logger } from '@sentry/utils';
import { GLOBAL_OBJ, logger } from '@sentry/utils';
import * as domain from 'domain';

import { init } from '../src/index.server';

const { Integrations } = SentryNode;

const global = getGlobalObject();

// normally this is set as part of the build process, so mock it here
(global as typeof global & { __rewriteFramesDistDir__: string }).__rewriteFramesDistDir__ = '.next';
(GLOBAL_OBJ as typeof GLOBAL_OBJ & { __rewriteFramesDistDir__: string }).__rewriteFramesDistDir__ = '.next';

const nodeInit = jest.spyOn(SentryNode, 'init');
const loggerLogSpy = jest.spyOn(logger, 'log');
Expand All @@ -23,7 +21,7 @@ function findIntegrationByName(integrations: Integration[] = [], name: string):
describe('Server init()', () => {
afterEach(() => {
jest.clearAllMocks();
global.__SENTRY__.hub = undefined;
GLOBAL_OBJ.__SENTRY__.hub = undefined;
delete process.env.VERCEL;
});

Expand Down
38 changes: 18 additions & 20 deletions packages/nextjs/test/performance/client.test.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
import { Transaction } from '@sentry/types';
import { getGlobalObject } from '@sentry/utils';
import { WINDOW } from '@sentry/utils';
import { JSDOM } from 'jsdom';
import { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils';
import { default as Router } from 'next/router';

import { nextRouterInstrumentation } from '../../src/performance/client';

const globalObject = getGlobalObject<
Window & {
__BUILD_MANIFEST?: {
sortedPages?: string[];
};
}
>();
const globalObject = WINDOW as typeof WINDOW & {
__BUILD_MANIFEST?: {
sortedPages?: string[];
};
};

const originalBuildManifest = globalObject.__BUILD_MANIFEST;
const originalBuildManifestRoutes = globalObject.__BUILD_MANIFEST?.sortedPages;
Expand Down Expand Up @@ -60,8 +58,8 @@ function createMockStartTransaction() {
}

describe('nextRouterInstrumentation', () => {
const originalGlobalDocument = getGlobalObject<Window>().document;
const originalGlobalLocation = getGlobalObject<Window>().location;
const originalGlobalDocument = WINDOW.document;
const originalGlobalLocation = WINDOW.location;

function setUpNextPage(pageProperties: {
url: string;
Expand Down Expand Up @@ -93,25 +91,25 @@ describe('nextRouterInstrumentation', () => {
// The Next.js routing instrumentations requires a few things to be present on pageload:
// 1. Access to window.document API for `window.document.getElementById`
// 2. Access to window.location API for `window.location.pathname`
Object.defineProperty(global, 'document', { value: dom.window.document, writable: true });
Object.defineProperty(global, 'location', { value: dom.window.document.location, writable: true });
Object.defineProperty(WINDOW, 'document', { value: dom.window.document, writable: true });
Object.defineProperty(WINDOW, 'location', { value: dom.window.document.location, writable: true });

// Define Next.js clientside build manifest with navigatable routes
(global as any).__BUILD_MANIFEST = {
...(global as any).__BUILD_MANIFEST,
sortedPages: pageProperties.navigatableRoutes,
globalObject.__BUILD_MANIFEST = {
...globalObject.__BUILD_MANIFEST,
sortedPages: pageProperties.navigatableRoutes as string[],
};
}

afterEach(() => {
// Clean up JSDom
Object.defineProperty(global, 'document', { value: originalGlobalDocument });
Object.defineProperty(global, 'location', { value: originalGlobalLocation });
Object.defineProperty(WINDOW, 'document', { value: originalGlobalDocument });
Object.defineProperty(WINDOW, 'location', { value: originalGlobalLocation });

// Reset Next.js' __BUILD_MANIFEST
(global as any).__BUILD_MANIFEST = originalBuildManifest;
if ((global as any).__BUILD_MANIFEST) {
(global as any).__BUILD_MANIFEST.sortedPages = originalBuildManifestRoutes;
globalObject.__BUILD_MANIFEST = originalBuildManifest;
if (globalObject.__BUILD_MANIFEST) {
globalObject.__BUILD_MANIFEST.sortedPages = originalBuildManifestRoutes as string[];
}

// Clear all event handlers
Expand Down
8 changes: 3 additions & 5 deletions packages/react/src/reactrouter.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Transaction, TransactionSource } from '@sentry/types';
import { getGlobalObject } from '@sentry/utils';
import { WINDOW } from '@sentry/utils';
import hoistNonReactStatics from 'hoist-non-react-statics';
import * as React from 'react';

Expand All @@ -26,8 +26,6 @@ export type RouteConfig = {
type MatchPath = (pathname: string, props: string | string[] | any, parent?: Match | null) => Match | null;
/* eslint-enable @typescript-eslint/no-explicit-any */

const global = getGlobalObject<Window>();

let activeTransaction: Transaction | undefined;

export function reactRouterV4Instrumentation(
Expand Down Expand Up @@ -57,8 +55,8 @@ function createReactRouterInstrumentation(
return history.location.pathname;
}

if (global && global.location) {
return global.location.pathname;
if (WINDOW && WINDOW.location) {
return WINDOW.location.pathname;
}

return undefined;
Expand Down
10 changes: 4 additions & 6 deletions packages/react/src/reactrouterv3.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Primitive, Transaction, TransactionContext, TransactionSource } from '@sentry/types';
import { getGlobalObject } from '@sentry/utils';
import { WINDOW } from '@sentry/utils';

import { Location, ReactRouterInstrumentation } from './types';

Expand All @@ -21,8 +21,6 @@ export type Match = (

type ReactRouterV3TransactionSource = Extract<TransactionSource, 'url' | 'route'>;

const global = getGlobalObject<Window>();

/**
* Creates routing instrumentation for React Router v3
* Works for React Router >= 3.2.0 and < 4.0.0
Expand All @@ -44,11 +42,11 @@ export function reactRouterV3Instrumentation(
let activeTransaction: Transaction | undefined;
let prevName: string | undefined;

// Have to use global.location because history.location might not be defined.
if (startTransactionOnPageLoad && global && global.location) {
// Have to use window.location because history.location might not be defined.
if (startTransactionOnPageLoad && WINDOW && WINDOW.location) {
normalizeTransactionName(
routes,
global.location as unknown as Location,
WINDOW.location as unknown as Location,
match,
(localName: string, source: ReactRouterV3TransactionSource = 'url') => {
prevName = localName;
Expand Down
6 changes: 2 additions & 4 deletions packages/react/src/reactrouterv6.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// https://gist.github.com/wontondon/e8c4bdf2888875e4c755712e99279536

import { Transaction, TransactionContext, TransactionSource } from '@sentry/types';
import { getGlobalObject, getNumberOfUrlSegments, logger } from '@sentry/utils';
import { getNumberOfUrlSegments, logger, WINDOW } from '@sentry/utils';
import hoistNonReactStatics from 'hoist-non-react-statics';
import React from 'react';

Expand Down Expand Up @@ -58,8 +58,6 @@ let _matchRoutes: MatchRoutes;
let _customStartTransaction: (context: TransactionContext) => Transaction | undefined;
let _startTransactionOnLocationChange: boolean;

const global = getGlobalObject<Window>();

const SENTRY_TAGS = {
'routing.instrumentation': 'react-router-v6',
};
Expand All @@ -76,7 +74,7 @@ export function reactRouterV6Instrumentation(
startTransactionOnPageLoad = true,
startTransactionOnLocationChange = true,
): void => {
const initPathName = global && global.location && global.location.pathname;
const initPathName = WINDOW && WINDOW.location && WINDOW.location.pathname;
if (startTransactionOnPageLoad && initPathName) {
activeTransaction = customStartTransaction({
name: initPathName,
Expand Down
8 changes: 3 additions & 5 deletions packages/remix/src/performance/client.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { ErrorBoundaryProps } from '@sentry/react';
import { withErrorBoundary } from '@sentry/react';
import { Transaction, TransactionContext } from '@sentry/types';
import { getGlobalObject, logger } from '@sentry/utils';
import { logger, WINDOW } from '@sentry/utils';
import * as React from 'react';

const DEFAULT_TAGS = {
Expand Down Expand Up @@ -38,11 +38,9 @@ let _useMatches: UseMatches;
let _customStartTransaction: (context: TransactionContext) => Transaction | undefined;
let _startTransactionOnLocationChange: boolean;

const global = getGlobalObject<Window>();

function getInitPathName(): string | undefined {
if (global && global.location) {
return global.location.pathname;
if (WINDOW && WINDOW.location) {
return WINDOW.location.pathname;
}

return undefined;
Expand Down
6 changes: 2 additions & 4 deletions packages/remix/test/index.client.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import { getCurrentHub } from '@sentry/core';
import * as SentryReact from '@sentry/react';
import { getGlobalObject } from '@sentry/utils';
import { GLOBAL_OBJ } from '@sentry/utils';

import { init } from '../src/index.client';

const global = getGlobalObject();

const reactInit = jest.spyOn(SentryReact, 'init');

describe('Client init()', () => {
afterEach(() => {
jest.clearAllMocks();
global.__SENTRY__.hub = undefined;
GLOBAL_OBJ.__SENTRY__.hub = undefined;
});

it('inits the React SDK', () => {
Expand Down
6 changes: 2 additions & 4 deletions packages/remix/test/index.server.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import * as SentryNode from '@sentry/node';
import { getCurrentHub } from '@sentry/node';
import { getGlobalObject } from '@sentry/utils';
import { GLOBAL_OBJ } from '@sentry/utils';

import { init } from '../src/index.server';

const global = getGlobalObject();

const nodeInit = jest.spyOn(SentryNode, 'init');

describe('Server init()', () => {
afterEach(() => {
jest.clearAllMocks();
global.__SENTRY__.hub = undefined;
GLOBAL_OBJ.__SENTRY__.hub = undefined;
});

it('inits the Node SDK', () => {
Expand Down
Loading