Skip to content

ref(replay): Use WINDOW instead of window #6316

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

Merged
merged 3 commits into from
Nov 29, 2022
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
2 changes: 0 additions & 2 deletions packages/replay/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ module.exports = {
rules: {
// TODO (high-prio): Go through console logs and figure out which ones should be replaced with the SDK logger
'no-console': 'off',
// TODO (high-pio): Re-enable this after migration
'no-restricted-globals': 'off',
// TODO (high-prio): Re-enable this after migration
'@typescript-eslint/explicit-member-accessibility': 'off',
// TODO (high-prio): Remove this exception from naming convention after migration
Expand Down
2 changes: 1 addition & 1 deletion packages/replay/config/rollup.config.core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const config = defineConfig({
format: 'esm',
},
],
external: [...Object.keys(pkg.dependencies || {})],
external: [...Object.keys(pkg.dependencies || {}), ...Object.keys(pkg.peerDependencies || {})],
plugins: [
typescript({
tsconfig: IS_PRODUCTION ? './config/tsconfig.core.json' : './tsconfig.json',
Expand Down
3 changes: 3 additions & 0 deletions packages/replay/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@
"lodash.debounce": "^4.0.8",
"rrweb": "^1.1.3"
},
"peerDependencies": {
"@sentry/browser": "7.22.0"
},
"engines": {
"node": ">=12"
},
Expand Down
3 changes: 2 additions & 1 deletion packages/replay/src/createPerformanceEntry.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { WINDOW } from '@sentry/browser';
import { browserPerformanceTimeOrigin } from '@sentry/utils';
import { record } from 'rrweb';

Expand Down Expand Up @@ -60,7 +61,7 @@ function createPerformanceEntry(entry: AllPerformanceEntry): ReplayPerformanceEn
function getAbsoluteTime(time: number): number {
// browserPerformanceTimeOrigin can be undefined if `performance` or
// `performance.now` doesn't exist, but this is already checked by this integration
return ((browserPerformanceTimeOrigin || window.performance.timeOrigin) + time) / 1000;
return ((browserPerformanceTimeOrigin || WINDOW.performance.timeOrigin) + time) / 1000;
}

// TODO: type definition!
Expand Down
1 change: 1 addition & 0 deletions packages/replay/src/eventBuffer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ interface CreateEventBufferParams {
}

export function createEventBuffer({ useCompression }: CreateEventBufferParams): IEventBuffer {
// eslint-disable-next-line no-restricted-globals
if (useCompression && window.Worker) {
const workerBlob = new Blob([workerString]);
const workerUrl = URL.createObjectURL(workerBlob);
Expand Down
27 changes: 14 additions & 13 deletions packages/replay/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable max-lines */ // TODO: We might want to split this file up
import { WINDOW } from '@sentry/browser';
import { addGlobalEventProcessor, getCurrentHub, Scope, setContext } from '@sentry/core';
import { Breadcrumb, Client, Event, Integration } from '@sentry/types';
import { addInstrumentationHandler, createEnvelope, logger } from '@sentry/utils';
Expand Down Expand Up @@ -232,7 +233,7 @@ export class Replay implements Integration {
return;
}
// XXX: See method comments above
window.setTimeout(() => this.start());
setTimeout(() => this.start());
}

/**
Expand Down Expand Up @@ -396,8 +397,8 @@ export class Replay implements Integration {
* first flush.
*/
setInitialState(): void {
const urlPath = `${window.location.pathname}${window.location.hash}${window.location.search}`;
const url = `${window.location.origin}${urlPath}`;
const urlPath = `${WINDOW.location.pathname}${WINDOW.location.hash}${WINDOW.location.search}`;
const url = `${WINDOW.location.origin}${urlPath}`;

this.performanceEvents = [];

Expand All @@ -414,9 +415,9 @@ export class Replay implements Integration {
*/
addListeners(): void {
try {
document.addEventListener('visibilitychange', this.handleVisibilityChange);
window.addEventListener('blur', this.handleWindowBlur);
window.addEventListener('focus', this.handleWindowFocus);
WINDOW.document.addEventListener('visibilitychange', this.handleVisibilityChange);
WINDOW.addEventListener('blur', this.handleWindowBlur);
WINDOW.addEventListener('focus', this.handleWindowFocus);

// There is no way to remove these listeners, so ensure they are only added once
if (!this.hasInitializedCoreListeners) {
Expand All @@ -440,7 +441,7 @@ export class Replay implements Integration {
}

// PerformanceObserver //
if (!('PerformanceObserver' in window)) {
if (!('PerformanceObserver' in WINDOW)) {
return;
}

Expand Down Expand Up @@ -475,10 +476,10 @@ export class Replay implements Integration {
*/
removeListeners(): void {
try {
document.removeEventListener('visibilitychange', this.handleVisibilityChange);
WINDOW.document.removeEventListener('visibilitychange', this.handleVisibilityChange);

window.removeEventListener('blur', this.handleWindowBlur);
window.removeEventListener('focus', this.handleWindowFocus);
WINDOW.removeEventListener('blur', this.handleWindowBlur);
WINDOW.removeEventListener('focus', this.handleWindowFocus);

if (this.performanceObserver) {
this.performanceObserver.disconnect();
Expand Down Expand Up @@ -665,7 +666,7 @@ export class Replay implements Integration {
* page will also trigger a change to a hidden state.
*/
handleVisibilityChange: () => void = () => {
if (document.visibilityState === 'visible') {
if (WINDOW.document.visibilityState === 'visible') {
this.doChangeToForegroundTasks();
} else {
this.doChangeToBackgroundTasks();
Expand Down Expand Up @@ -980,13 +981,13 @@ export class Replay implements Integration {
addMemoryEntry(): Promise<void[]> | undefined {
// window.performance.memory is a non-standard API and doesn't work on all browsers
// so we check before creating the event.
if (!('memory' in window.performance)) {
if (!('memory' in WINDOW.performance)) {
return;
}

return this.createPerformanceSpans([
// @ts-ignore memory doesn't exist on type Performance as the API is non-standard (we check that it exists above)
createMemoryEntry(window.performance.memory),
createMemoryEntry(WINDOW.performance.memory),
]);
}

Expand Down
6 changes: 4 additions & 2 deletions packages/replay/src/session/deleteSession.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
import { WINDOW } from '@sentry/browser';

import { REPLAY_SESSION_KEY } from './constants';

/**
* Deletes a session from storage
*/
export function deleteSession(): void {
const hasSessionStorage = 'sessionStorage' in window;
const hasSessionStorage = 'sessionStorage' in WINDOW;

if (!hasSessionStorage) {
return;
}

try {
window.sessionStorage.removeItem(REPLAY_SESSION_KEY);
WINDOW.sessionStorage.removeItem(REPLAY_SESSION_KEY);
} catch {
// Ignore potential SecurityError exceptions
}
Expand Down
6 changes: 4 additions & 2 deletions packages/replay/src/session/fetchSession.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { WINDOW } from '@sentry/browser';

import { SampleRates } from '../types';
import { REPLAY_SESSION_KEY } from './constants';
import { Session } from './Session';
Expand All @@ -6,15 +8,15 @@ import { Session } from './Session';
* Fetches a session from storage
*/
export function fetchSession({ sessionSampleRate, errorSampleRate }: SampleRates): Session | null {
const hasSessionStorage = 'sessionStorage' in window;
const hasSessionStorage = 'sessionStorage' in WINDOW;

if (!hasSessionStorage) {
return null;
}

try {
// This can throw if cookies are disabled
const sessionStringFromStorage = window.sessionStorage.getItem(REPLAY_SESSION_KEY);
const sessionStringFromStorage = WINDOW.sessionStorage.getItem(REPLAY_SESSION_KEY);

if (!sessionStringFromStorage) {
return null;
Expand Down
6 changes: 4 additions & 2 deletions packages/replay/src/session/saveSession.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import { WINDOW } from '@sentry/browser';

import { REPLAY_SESSION_KEY } from './constants';
import { Session } from './Session';

export function saveSession(session: Session): void {
const hasSessionStorage = 'sessionStorage' in window;
const hasSessionStorage = 'sessionStorage' in WINDOW;
if (!hasSessionStorage) {
return;
}

try {
window.sessionStorage.setItem(REPLAY_SESSION_KEY, JSON.stringify(session));
WINDOW.sessionStorage.setItem(REPLAY_SESSION_KEY, JSON.stringify(session));
} catch {
// Ignore potential SecurityError exceptions
}
Expand Down
1 change: 1 addition & 0 deletions packages/replay/src/util/isBrowser.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { isNodeEnv } from '@sentry/utils';

export function isBrowser(): boolean {
// eslint-disable-next-line no-restricted-globals
return typeof window !== 'undefined' && !isNodeEnv();
}
4 changes: 3 additions & 1 deletion packages/replay/src/util/isInternal.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { WINDOW } from '@sentry/browser';

import { isBrowser } from './isBrowser';

/**
* Returns true if we are currently recording an internal to Sentry replay
* (e.g. on https://sentry.io )
*/
export function isInternal(): boolean {
return isBrowser() && ['sentry.io', 'dev.getsentry.net'].includes(window.location.host);
return isBrowser() && ['sentry.io', 'dev.getsentry.net'].includes(WINDOW.location.host);
}
21 changes: 10 additions & 11 deletions packages/replay/test/unit/flush.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { WINDOW } from '@sentry/browser';
import * as SentryUtils from '@sentry/utils';
import { BASE_TIMESTAMP, mockRrweb, mockSdk } from '@test';

Expand All @@ -20,7 +21,7 @@ type MockEventBufferFinish = jest.MockedFunction<Exclude<typeof Replay.prototype
type MockFlush = jest.MockedFunction<typeof Replay.prototype.flush>;
type MockRunFlush = jest.MockedFunction<typeof Replay.prototype.runFlush>;

const prevLocation = window.location;
const prevLocation = WINDOW.location;
let domHandler: (args: any) => any;

const { record: mockRecord } = mockRrweb();
Expand Down Expand Up @@ -91,9 +92,7 @@ afterEach(async () => {
replay.clearSession();
replay.loadSession({ expiry: SESSION_IDLE_DURATION });
mockRecord.takeFullSnapshot.mockClear();
// @ts-ignore: The operand of a 'delete' operator must be optional.ts(2790)
delete window.location;
Object.defineProperty(window, 'location', {
Object.defineProperty(WINDOW, 'location', {
value: prevLocation,
writable: true,
});
Expand All @@ -109,10 +108,10 @@ it('flushes twice after multiple flush() calls)', async () => {
// the following blur events will all call a debounced flush function, which
// should end up queueing a second flush

window.dispatchEvent(new Event('blur'));
window.dispatchEvent(new Event('blur'));
window.dispatchEvent(new Event('blur'));
window.dispatchEvent(new Event('blur'));
WINDOW.dispatchEvent(new Event('blur'));
WINDOW.dispatchEvent(new Event('blur'));
WINDOW.dispatchEvent(new Event('blur'));
WINDOW.dispatchEvent(new Event('blur'));

expect(replay.flush).toHaveBeenCalledTimes(4);

Expand All @@ -138,7 +137,7 @@ it('long first flush enqueues following events', async () => {
expect(mockAddPerformanceEntries).not.toHaveBeenCalled();

// flush #1 @ t=0s - due to blur
window.dispatchEvent(new Event('blur'));
WINDOW.dispatchEvent(new Event('blur'));
expect(replay.flush).toHaveBeenCalledTimes(1);
expect(replay.runFlush).toHaveBeenCalledTimes(1);

Expand All @@ -152,7 +151,7 @@ it('long first flush enqueues following events', async () => {

await advanceTimers(1000);
// flush #3 @ t=6s - due to blur
window.dispatchEvent(new Event('blur'));
WINDOW.dispatchEvent(new Event('blur'));
expect(replay.flush).toHaveBeenCalledTimes(3);

// NOTE: Blur also adds a breadcrumb which calls `addUpdate`, meaning it will
Expand All @@ -161,7 +160,7 @@ it('long first flush enqueues following events', async () => {
expect(replay.flush).toHaveBeenCalledTimes(3);

// flush #4 @ t=14s - due to blur
window.dispatchEvent(new Event('blur'));
WINDOW.dispatchEvent(new Event('blur'));
expect(replay.flush).toHaveBeenCalledTimes(4);

expect(replay.runFlush).toHaveBeenCalledTimes(1);
Expand Down
4 changes: 2 additions & 2 deletions packages/replay/test/unit/index-errorSampleRate.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
jest.unmock('@sentry/browser');

import { captureException } from '@sentry/browser';
import { captureException, WINDOW } from '@sentry/browser';
import { BASE_TIMESTAMP, RecordMock } from '@test';
import { PerformanceEntryResource } from '@test/fixtures/performanceEntry/resource';
import { resetSdkMock } from '@test/mocks';
Expand Down Expand Up @@ -317,7 +317,7 @@ describe('Replay (errorSampleRate)', () => {
*/
it('sends a replay after loading the session multiple times', async () => {
// Pretend that a session is already saved before loading replay
window.sessionStorage.setItem(
WINDOW.sessionStorage.setItem(
REPLAY_SESSION_KEY,
`{"segmentId":0,"id":"fd09adfc4117477abc8de643e5a5798a","sampled":"error","started":${BASE_TIMESTAMP},"lastActivity":${BASE_TIMESTAMP}}`,
);
Expand Down
Loading