From 555aba654cedfe0141bc08161cf2e77dedcaf589 Mon Sep 17 00:00:00 2001 From: Vladimir Date: Sun, 8 May 2022 08:34:37 +0300 Subject: [PATCH] fix: calling global functions in happy-dom, refactor sharing global state (#1262) * chore: top/parent also the share the same state with globalThis * fix: can call global functions without explicit this * chore: move shouldBind check outside of defineProperty * chore: cleanup --- .../src/integrations/chai/jest-expect.ts | 6 +- .../vitest/src/integrations/env/happy-dom.ts | 2 +- packages/vitest/src/integrations/env/utils.ts | 110 ++++++++++-------- test/core/test/dom.test.ts | 90 +++++++++++++- test/core/test/happy-dom.test.ts | 13 ++- 5 files changed, 169 insertions(+), 52 deletions(-) diff --git a/packages/vitest/src/integrations/chai/jest-expect.ts b/packages/vitest/src/integrations/chai/jest-expect.ts index 0cad69e136b7..df1bd4c35b4a 100644 --- a/packages/vitest/src/integrations/chai/jest-expect.ts +++ b/packages/vitest/src/integrations/chai/jest-expect.ts @@ -20,7 +20,7 @@ if (!Object.prototype.hasOwnProperty.call(global, MATCHERS_OBJECT)) { expectedAssertionsNumber: null, expectedAssertionsNumberErrorGen: null, } - Object.defineProperty(global, MATCHERS_OBJECT, { + Object.defineProperty(globalThis, MATCHERS_OBJECT, { value: { state: defaultState, }, @@ -28,12 +28,12 @@ if (!Object.prototype.hasOwnProperty.call(global, MATCHERS_OBJECT)) { } export const getState = (): State => - (global as any)[MATCHERS_OBJECT].state + (globalThis as any)[MATCHERS_OBJECT].state export const setState = ( state: Partial, ): void => { - Object.assign((global as any)[MATCHERS_OBJECT].state, state) + Object.assign((globalThis as any)[MATCHERS_OBJECT].state, state) } // Jest Expect Compact diff --git a/packages/vitest/src/integrations/env/happy-dom.ts b/packages/vitest/src/integrations/env/happy-dom.ts index deeb328181dc..b8232bc5cab8 100644 --- a/packages/vitest/src/integrations/env/happy-dom.ts +++ b/packages/vitest/src/integrations/env/happy-dom.ts @@ -10,7 +10,7 @@ export default ({ const { Window, GlobalWindow } = await importModule('happy-dom') as typeof import('happy-dom') const win = new (GlobalWindow || Window)() - const { keys, allowRewrite } = populateGlobal(global, win) + const { keys, allowRewrite } = populateGlobal(global, win, { bindFunctions: true }) const originals = new Map( allowRewrite.map(([key]) => [key, global[key]]), diff --git a/packages/vitest/src/integrations/env/utils.ts b/packages/vitest/src/integrations/env/utils.ts index cd971cfb7879..efe4b597d91a 100644 --- a/packages/vitest/src/integrations/env/utils.ts +++ b/packages/vitest/src/integrations/env/utils.ts @@ -8,6 +8,8 @@ const allowRewrite = [ const skipKeys = [ 'window', 'self', + 'top', + 'parent', ] export function getWindowKeys(global: any, win: any) { @@ -24,15 +26,23 @@ export function getWindowKeys(global: any, win: any) { return keys } -export function populateGlobal(global: any, win: any) { +interface PopulateOptions { + bindFunctions?: boolean +} + +export function populateGlobal(global: any, win: any, options: PopulateOptions = {}) { + const { bindFunctions = false } = options const keys = getWindowKeys(global, win) const overrideObject = new Map() for (const key of keys) { + const shouldBind = bindFunctions && typeof win[key] === 'function' Object.defineProperty(global, key, { get() { if (overrideObject.has(key)) return overrideObject.get(key) + if (shouldBind) + return win[key].bind(win) return win[key] }, set(v) { @@ -42,63 +52,66 @@ export function populateGlobal(global: any, win: any) { }) } - const globalKeys = new Set(['window', 'self', 'GLOBAL', 'global']) + const globalKeys = new Set(['window', 'self', 'top', 'parent']) + + // we are creating a proxy that intercepts all access to the global object, + // stores new value on `override`, and returns only these values, + // so it actually shares only values defined inside tests + const globalProxy = new Proxy(win.window, { + get(target, p, receiver) { + if (overrideObject.has(p)) + return overrideObject.get(p) + return Reflect.get(target, p, receiver) + }, + set(target, p, value, receiver) { + try { + // if property is defined with "configurable: false", + // this will throw an error, but `self.prop = value` should not throw + // this matches browser behaviour where it silently ignores the error + // and returns previously defined value, which is a hell for debugging + Object.defineProperty(global, p, { + get: () => overrideObject.get(p), + set: value => overrideObject.set(p, value), + configurable: true, + }) + overrideObject.set(p, value) + Reflect.set(target, p, value, receiver) + } + catch { + // ignore + } + return true + }, + deleteProperty(target, p) { + Reflect.deleteProperty(global, p) + overrideObject.delete(p) + return Reflect.deleteProperty(target, p) + }, + defineProperty(target, p, attributes) { + if (attributes.writable && 'value' in attributes) { + // skip - already covered by "set" + } + else if (attributes.get) { + overrideObject.delete(p) + Reflect.defineProperty(global, p, attributes) + } + return Reflect.defineProperty(target, p, attributes) + }, + }) globalKeys.forEach((key) => { if (!win[key]) return - const proxy = new Proxy(win[key], { - get(target, p, receiver) { - if (overrideObject.has(p)) - return overrideObject.get(p) - return Reflect.get(target, p, receiver) - }, - set(target, p, value, receiver) { - try { - // if property is defined with configurable: false, - // this will throw an error, but `self.prop = value` should not throw - // this matches browser behaviour where it silently ignores the error - // and returns previously defined value, which is a hell for debugging - Object.defineProperty(global, p, { - get: () => overrideObject.get(p), - set: value => overrideObject.set(p, value), - configurable: true, - }) - overrideObject.set(p, value) - Reflect.set(target, p, value, receiver) - } - catch { - // ignore - } - return true - }, - deleteProperty(target, p) { - Reflect.deleteProperty(global, p) - overrideObject.delete(p) - return Reflect.deleteProperty(target, p) - }, - defineProperty(target, p, attributes) { - if (attributes.writable && 'value' in attributes) { - // skip - already covered by "set" - } - else if (attributes.get) { - overrideObject.delete(p) - Reflect.defineProperty(global, p, attributes) - } - return Reflect.defineProperty(target, p, attributes) - }, - }) - Object.defineProperty(global, key, { get() { - return proxy + return globalProxy }, configurable: true, }) }) - global.globalThis = new Proxy(global.globalThis, { + const globalThisProxy = new Proxy(global.globalThis, { set(target, key, value, receiver) { overrideObject.set(key, value) return Reflect.set(target, key, value, receiver) @@ -121,6 +134,11 @@ export function populateGlobal(global: any, win: any) { }, }) + global.globalThis = globalThisProxy + + if (global.global) + global.global = globalThisProxy + skipKeys.forEach(k => keys.add(k)) return { diff --git a/test/core/test/dom.test.ts b/test/core/test/dom.test.ts index 49e7bafeb618..94cb8fd6ff43 100644 --- a/test/core/test/dom.test.ts +++ b/test/core/test/dom.test.ts @@ -1,7 +1,7 @@ /** * @vitest-environment jsdom */ -import { expect, it } from 'vitest' +import { expect, it, vi } from 'vitest' it('jsdom', () => { expect(window).toBeDefined() @@ -24,3 +24,91 @@ it('Image works as expected', () => { expect(img.width).toBe(100) }) + +it('defined on self/window are defined on global', () => { + expect(self).toBeDefined() + expect(window).toBeDefined() + + expect(self.__property).not.toBeDefined() + expect(window.__property).not.toBeDefined() + expect(globalThis.__property).not.toBeDefined() + + globalThis.__property = 'defined_value' + + expect(__property).toBe('defined_value') + expect(self.__property).toBe('defined_value') + expect(window.__property).toBe('defined_value') + expect(globalThis.__property).toBe('defined_value') + + self.__property = 'test_value' + + expect(__property).toBe('test_value') + expect(self.__property).toBe('test_value') + expect(window.__property).toBe('test_value') + expect(globalThis.__property).toBe('test_value') + + window.__property = 'new_value' + + expect(__property).toBe('new_value') + expect(self.__property).toBe('new_value') + expect(window.__property).toBe('new_value') + expect(globalThis.__property).toBe('new_value') + + globalThis.__property = 'global_value' + + expect(__property).toBe('global_value') + expect(self.__property).toBe('global_value') + expect(window.__property).toBe('global_value') + expect(globalThis.__property).toBe('global_value') + + const obj = {} + + self.__property = obj + + expect(self.__property).toBe(obj) + expect(window.__property).toBe(obj) + expect(globalThis.__property).toBe(obj) +}) + +it('usage with defineProperty', () => { + Object.defineProperty(self, '__property', { + get: () => 'self_property', + configurable: true, + }) + + expect(__property).toBe('self_property') + expect(self.__property).toBe('self_property') + expect(globalThis.__property).toBe('self_property') + expect(window.__property).toBe('self_property') + + Object.defineProperty(window, '__property', { + get: () => 'window_property', + configurable: true, + }) + + expect(__property).toBe('window_property') + expect(self.__property).toBe('window_property') + expect(globalThis.__property).toBe('window_property') + expect(window.__property).toBe('window_property') + + Object.defineProperty(globalThis, '__property', { + get: () => 'global_property', + configurable: true, + }) + + expect(__property).toBe('global_property') + expect(self.__property).toBe('global_property') + expect(globalThis.__property).toBe('global_property') + expect(window.__property).toBe('global_property') +}) + +it('can call global functions without window works as expected', async () => { + const noop = vi.fn() + + expect(() => addEventListener('abort', noop)).not.toThrow() + expect(() => scrollTo()).not.toThrow() + expect(() => requestAnimationFrame(noop)).not.toThrow() + expect(() => window.requestAnimationFrame(noop)).not.toThrow() + expect(() => self.requestAnimationFrame(noop)).not.toThrow() + expect(() => globalThis.requestAnimationFrame(noop)).not.toThrow() +}) diff --git a/test/core/test/happy-dom.test.ts b/test/core/test/happy-dom.test.ts index add3b597ff4d..e370f314aac0 100644 --- a/test/core/test/happy-dom.test.ts +++ b/test/core/test/happy-dom.test.ts @@ -4,7 +4,7 @@ /* eslint-disable vars-on-top */ -import { expect, it } from 'vitest' +import { expect, it, vi } from 'vitest' declare global { // eslint-disable-next-line no-var @@ -87,3 +87,14 @@ it('usage with defineProperty', () => { expect(globalThis.__property).toBe('global_property') expect(window.__property).toBe('global_property') }) + +it('can call global functions without window works as expected', async () => { + const noop = vi.fn() + + expect(() => addEventListener('abort', noop)).not.toThrow() + expect(() => scrollTo()).not.toThrow() + expect(() => requestAnimationFrame(noop)).not.toThrow() + expect(() => window.requestAnimationFrame(noop)).not.toThrow() + expect(() => self.requestAnimationFrame(noop)).not.toThrow() + expect(() => globalThis.requestAnimationFrame(noop)).not.toThrow() +})