diff --git a/packages/browser-utils/src/instrument/history.ts b/packages/browser-utils/src/instrument/history.ts index 8494fca3076e..60ee888aae24 100644 --- a/packages/browser-utils/src/instrument/history.ts +++ b/packages/browser-utils/src/instrument/history.ts @@ -18,7 +18,10 @@ export function addHistoryInstrumentationHandler(handler: (data: HandlerDataHist maybeInstrument(type, instrumentHistory); } -function instrumentHistory(): void { +/** + * Exported just for testing + */ +export function instrumentHistory(): void { // The `popstate` event may also be triggered on `pushState`, but it may not always reliably be emitted by the browser // Which is why we also monkey-patch methods below, in addition to this WINDOW.addEventListener('popstate', () => { diff --git a/packages/browser-utils/test/instrument/history.test.ts b/packages/browser-utils/test/instrument/history.test.ts new file mode 100644 index 000000000000..9d9ffd5b0686 --- /dev/null +++ b/packages/browser-utils/test/instrument/history.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it, vi } from 'vitest'; +import { WINDOW } from '../../src/types'; +import { afterEach } from 'node:test'; + +import { instrumentHistory } from './../../src/instrument/history'; + +describe('instrumentHistory', () => { + const originalHistory = WINDOW.history; + WINDOW.addEventListener = vi.fn(); + + afterEach(() => { + // @ts-expect-error - this is fine for testing + WINDOW.history = originalHistory; + }); + + it("doesn't throw if history is not available", () => { + // @ts-expect-error - this is fine for testing + WINDOW.history = undefined; + expect(instrumentHistory).not.toThrow(); + expect(WINDOW.history).toBe(undefined); + }); + + it('adds an event listener for popstate', () => { + // adds an event listener for popstate + expect(WINDOW.addEventListener).toHaveBeenCalledTimes(1); + expect(WINDOW.addEventListener).toHaveBeenCalledWith('popstate', expect.any(Function)); + }); + + it("doesn't throw if history.pushState is not a function", () => { + // @ts-expect-error - only partially adding history properties + WINDOW.history = { + replaceState: () => {}, + pushState: undefined, + }; + + expect(instrumentHistory).not.toThrow(); + + expect(WINDOW.history).toEqual({ + replaceState: expect.any(Function), // patched function + pushState: undefined, // unpatched + }); + }); + + it("doesn't throw if history.replaceState is not a function", () => { + // @ts-expect-error - only partially adding history properties + WINDOW.history = { + replaceState: undefined, + pushState: () => {}, + }; + + expect(instrumentHistory).not.toThrow(); + + expect(WINDOW.history).toEqual({ + replaceState: undefined, // unpatched + pushState: expect.any(Function), // patched function + }); + }); +}); diff --git a/packages/core/src/utils-hoist/object.ts b/packages/core/src/utils-hoist/object.ts index 31d1d862d01f..2c9080682a9d 100644 --- a/packages/core/src/utils-hoist/object.ts +++ b/packages/core/src/utils-hoist/object.ts @@ -10,6 +10,8 @@ import { truncate } from './string'; /** * Replace a method in an object with a wrapped version of itself. * + * If the method on the passed object is not a function, the wrapper will not be applied. + * * @param source An object that contains a method to be wrapped. * @param name The name of the method to be wrapped. * @param replacementFactory A higher-order function that takes the original version of the given method and returns a @@ -23,7 +25,13 @@ export function fill(source: { [key: string]: any }, name: string, replacementFa return; } - const original = source[name] as () => any; + // explicitly casting to unknown because we don't know the type of the method initially at all + const original = source[name] as unknown; + + if (typeof original !== 'function') { + return; + } + const wrapped = replacementFactory(original) as WrappedFunction; // Make sure it's a function first, as we need to attach an empty prototype for `defineProperties` to work diff --git a/packages/core/src/utils-hoist/supports.ts b/packages/core/src/utils-hoist/supports.ts index d52c06e702ea..e486d672a625 100644 --- a/packages/core/src/utils-hoist/supports.ts +++ b/packages/core/src/utils-hoist/supports.ts @@ -61,7 +61,7 @@ export function supportsDOMException(): boolean { * @returns Answer to the given question. */ export function supportsHistory(): boolean { - return 'history' in WINDOW; + return 'history' in WINDOW && !!WINDOW.history; } /** diff --git a/packages/core/test/utils-hoist/object.test.ts b/packages/core/test/utils-hoist/object.test.ts index 668d071108f9..e1c32f163193 100644 --- a/packages/core/test/utils-hoist/object.test.ts +++ b/packages/core/test/utils-hoist/object.test.ts @@ -54,6 +54,19 @@ describe('fill()', () => { expect(source.prop()).toEqual(41); }); + test.each([42, null, undefined, {}])("does't throw if the property is not a function but %s", (propValue: any) => { + const source = { + foo: propValue, + }; + const name = 'foo'; + const replacement = vi.fn().mockImplementationOnce(cb => cb); + + fill(source, name, replacement); + + expect(source.foo).toBe(propValue); + expect(replacement).not.toBeCalled(); + }); + test('can do anything inside replacement function', () => { const source = { foo: (): number => 42, diff --git a/packages/core/test/utils-hoist/supports.test.ts b/packages/core/test/utils-hoist/supports.test.ts new file mode 100644 index 000000000000..b2b444ab9b99 --- /dev/null +++ b/packages/core/test/utils-hoist/supports.test.ts @@ -0,0 +1,32 @@ +import { afterEach } from 'node:test'; +import { supportsHistory } from '../../src/utils-hoist/supports'; +import { describe, it, expect } from 'vitest'; + +describe('supportsHistory', () => { + const originalHistory = globalThis.history; + + afterEach(() => { + globalThis.history = originalHistory; + }); + + it('returns true if history is available', () => { + // @ts-expect-error - not setting all history properties + globalThis.history = { + pushState: () => {}, + replaceState: () => {}, + }; + expect(supportsHistory()).toBe(true); + }); + + it('returns false if history is not available', () => { + // @ts-expect-error - deletion is okay in this case + delete globalThis.history; + expect(supportsHistory()).toBe(false); + }); + + it('returns false if history is undefined', () => { + // @ts-expect-error - undefined is okay in this case + globalThis.history = undefined; + expect(supportsHistory()).toBe(false); + }); +});