From 227b9a74bd4558174fd911f8c05b8290fe606052 Mon Sep 17 00:00:00 2001 From: David Ortner Date: Wed, 20 Mar 2024 21:48:30 +0100 Subject: [PATCH] fix: [#1339] Fixes problem with properties defined as getters not being registered globally by Vitest --- packages/global-registrator/package.json | 6 +- .../src/GlobalRegistrator.ts | 29 +-- .../test/react/React.test.tsx | 209 +++++++++++++----- packages/happy-dom/package.json | 4 +- packages/happy-dom/src/window/GlobalWindow.ts | 57 +++++ .../test/window/GlobalWindow.test.ts | 33 +++ 6 files changed, 255 insertions(+), 83 deletions(-) diff --git a/packages/global-registrator/package.json b/packages/global-registrator/package.json index 06f3e9989..756dac107 100644 --- a/packages/global-registrator/package.json +++ b/packages/global-registrator/package.json @@ -65,8 +65,10 @@ "access": "public" }, "scripts": { - "compile": "rm -rf lib cjs && tsc && tsc --moduleResolution Node --module CommonJS --outDir cjs && npm run change-cjs-file-extension", - "change-cjs-file-extension": "node ../happy-dom/bin/change-file-extension.cjs --dir=./cjs --fromExt=.js --toExt=.cjs", + "compile": "npm run compile:esm && npm run compile:cjs", + "compile:esm": "tsc", + "compile:cjs": "rm -rf cjs && tsc --moduleResolution Node --module CommonJS --outDir cjs && npm run compile:change-cjs-file-extension", + "compile:change-cjs-file-extension": "node ../happy-dom/bin/change-file-extension.cjs --dir=./cjs --fromExt=.js --toExt=.cjs", "watch": "npm run compile && tsc -w --preserveWatchOutput", "test": "rm -rf tmp && tsc --project ./test && node ../happy-dom/bin/change-file-extension.cjs --dir=./tmp --fromExt=.js --toExt=.cjs && node ./tmp/react/React.test.cjs", "test:debug": "tsc --project ./test && node ../happy-dom/bin/change-file-extension.cjs --dir=./tmp --fromExt=.js --toExt=.cjs && node --inspect-brk ./tmp/react/React.test.cjs" diff --git a/packages/global-registrator/src/GlobalRegistrator.ts b/packages/global-registrator/src/GlobalRegistrator.ts index e19dd0931..525f1e1d2 100644 --- a/packages/global-registrator/src/GlobalRegistrator.ts +++ b/packages/global-registrator/src/GlobalRegistrator.ts @@ -1,4 +1,4 @@ -import { GlobalWindow, Window, EventTarget } from 'happy-dom'; +import { GlobalWindow } from 'happy-dom'; import type { IOptionalBrowserSettings } from 'happy-dom'; const IGNORE_LIST = ['constructor', 'undefined', 'NaN', 'global', 'globalThis']; @@ -41,8 +41,8 @@ export default class GlobalRegistrator { const globalPropertyDescriptor = Object.getOwnPropertyDescriptor(global, key); if ( - windowPropertyDescriptor.value !== undefined && - (!globalPropertyDescriptor || + !globalPropertyDescriptor || + (windowPropertyDescriptor.value !== undefined && windowPropertyDescriptor.value !== globalPropertyDescriptor.value) ) { this.registered[key] = globalPropertyDescriptor || null; @@ -62,29 +62,6 @@ export default class GlobalRegistrator { } } - for (const windowClass of [GlobalWindow, Window, EventTarget]) { - const propertyDescriptors = Object.getOwnPropertyDescriptors( - Reflect.getPrototypeOf(windowClass.prototype) - ); - for (const key of Object.keys(propertyDescriptors)) { - if (!IGNORE_LIST.includes(key) && !this.registered[key]) { - const windowPropertyDescriptor = propertyDescriptors[key]; - if (windowPropertyDescriptor.get || windowPropertyDescriptor.set) { - const globalPropertyDescriptor = Object.getOwnPropertyDescriptor(global, key); - - this.registered[key] = globalPropertyDescriptor || null; - - Object.defineProperty(global, key, { - configurable: true, - enumerable: windowPropertyDescriptor.enumerable, - get: windowPropertyDescriptor.get?.bind(window), - set: windowPropertyDescriptor.set?.bind(window) - }); - } - } - } - } - for (const key of SELF_REFERRING) { this.registered[key] = null; global[key] = global; diff --git a/packages/global-registrator/test/react/React.test.tsx b/packages/global-registrator/test/react/React.test.tsx index 1f60bdbc5..f612bb8d4 100644 --- a/packages/global-registrator/test/react/React.test.tsx +++ b/packages/global-registrator/test/react/React.test.tsx @@ -4,94 +4,197 @@ import ReactDOM from 'react-dom/client'; import { act } from 'react-dom/test-utils'; import ReactComponent from './ReactComponent.js'; +const GETTERS = [ + 'location', + 'history', + 'navigator', + 'screen', + 'sessionStorage', + 'localStorage', + 'opener', + 'scrollX', + 'pageXOffset', + 'scrollY', + 'pageYOffset', + 'CSS', + 'innerWidth', + 'innerHeight', + 'outerWidth', + 'outerHeight', + 'devicePixelRatio' +]; + async function main(): Promise { const selfReferringProperties = ['self', 'top', 'parent', 'window']; // eslint-disable-next-line @typescript-eslint/consistent-type-assertions const originalSetTimeout = global.setTimeout; + /** + * Registers Happy DOM globally. + */ GlobalRegistrator.register(); - const appElement = document.createElement('app'); - let root; - document.body.appendChild(appElement); + /** + * Test if all properties defined as getter are included in the global object. + */ + function testGetters(): void { + const included: string[] = []; + const propertyNames = Object.getOwnPropertyNames(global); + for (const name of GETTERS) { + if (propertyNames.includes(name)) { + included.push(name); + } + } - async function mountReactComponent(): Promise { - act(() => { - root = ReactDOM.createRoot(appElement); - root.render(); - }); + if (included.length !== GETTERS.length) { + throw Error( + 'Object.getOwnPropertyNames() did not return all properties defined as getter. Expected: ' + + GETTERS.join(', ') + + '. Got: ' + + included.join(', ') + + '.' + ); + } + } + + testGetters(); + + /** + * Test if it is possible to create a React component and mount it. + */ + async function testReactComponent(): Promise { + const appElement = document.createElement('app'); + let root; + document.body.appendChild(appElement); + + async function mountReactComponent(): Promise { + act(() => { + root = ReactDOM.createRoot(appElement); + root.render(); + }); - await new Promise((resolve) => setTimeout(resolve, 2)); + await new Promise((resolve) => setTimeout(resolve, 2)); - if (appElement.innerHTML !== '
Test
') { - throw Error('React not rendered correctly.'); + if (appElement.innerHTML !== '
Test
') { + throw Error('React not rendered correctly.'); + } } - } - function unmountReactComponent(): void { - act(() => { - root.unmount(); - }); + function unmountReactComponent(): void { + act(() => { + root.unmount(); + }); - if (appElement.innerHTML !== '') { - throw Error('React not unmounted correctly.'); + if (appElement.innerHTML !== '') { + throw Error('React not unmounted correctly.'); + } } - } - if (global.setTimeout === originalSetTimeout) { - throw Error('Happy DOM function not registered.'); - } + if (global.setTimeout === originalSetTimeout) { + throw Error('Happy DOM function not registered.'); + } - for (const property of selfReferringProperties) { - if (global[property] !== global) { - throw Error('Self referring property property was not registered.'); + for (const property of selfReferringProperties) { + if (global[property] !== global) { + throw Error('Self referring property property was not registered.'); + } } + + await mountReactComponent(); + unmountReactComponent(); } - /** @see https://github.com/capricorn86/happy-dom/issues/1230 */ - globalThis.location.href = 'https://example.com/'; - if (globalThis.location.href !== 'https://example.com/') { - throw Error('The property "location.href" could not be set.'); + await testReactComponent(); + + /** + * Test if it is possible to set the location.href property and that location isn't replaced to a new object. + * + * @see https://github.com/capricorn86/happy-dom/issues/1230 + * */ + function testLocationHref(): void { + globalThis.location.href = 'https://example.com/'; + if (globalThis.location.href !== 'https://example.com/') { + throw Error('The property "location.href" could not be set.'); + } } - await mountReactComponent(); - unmountReactComponent(); + testLocationHref(); + /** + * Unregisters Happy DOM globally. + */ GlobalRegistrator.unregister(); - if (global.setTimeout !== originalSetTimeout) { - throw Error('Global property was not restored.'); - } - - GlobalRegistrator.register({ - url: 'https://example.com/', - width: 1920, - height: 1080, - settings: { - navigator: { - userAgent: 'Custom User Agent' + /** + * Test if all properties defined as getter are removed from the global object. + */ + function testGettersAfterUnregister(): void { + const included: string[] = []; + const propertyNames = Object.getOwnPropertyNames(global); + for (const name of GETTERS) { + if (propertyNames.includes(name)) { + included.push(name); } } - }); - if (globalThis.location.href !== 'https://example.com/') { - throw Error('The option "url" has no affect.'); + if (included.length !== 0) { + throw Error( + 'Object.getOwnPropertyNames() did not remove all properties defined as getter. Expected: []. Got: ' + + included.join(', ') + + '.' + ); + } } - if (globalThis.innerWidth !== 1920) { - throw Error('The option "width" has no affect.'); - } + testGettersAfterUnregister(); - if (globalThis.innerHeight !== 1080) { - throw Error('The option "height" has no affect.'); + /** + * Test if setTimeout is restored. + */ + function testSetTimeout(): void { + if (global.setTimeout !== originalSetTimeout) { + throw Error('Global property was not restored.'); + } } - if (globalThis.navigator.userAgent !== 'Custom User Agent') { - throw Error('The option "settings.userAgent" has no affect.'); + testSetTimeout(); + + /** + * Test registering with options. + */ + function testWindowOptions(): void { + GlobalRegistrator.register({ + url: 'https://example.com/', + width: 1920, + height: 1080, + settings: { + navigator: { + userAgent: 'Custom User Agent' + } + } + }); + + if (globalThis.location.href !== 'https://example.com/') { + throw Error('The option "url" has no affect.'); + } + + if (globalThis.innerWidth !== 1920) { + throw Error('The option "width" has no affect.'); + } + + if (globalThis.innerHeight !== 1080) { + throw Error('The option "height" has no affect.'); + } + + if (globalThis.navigator.userAgent !== 'Custom User Agent') { + throw Error('The option "settings.userAgent" has no affect.'); + } + + GlobalRegistrator.unregister(); } - GlobalRegistrator.unregister(); + testWindowOptions(); } main(); diff --git a/packages/happy-dom/package.json b/packages/happy-dom/package.json index b1d76bf24..66dd82d3c 100644 --- a/packages/happy-dom/package.json +++ b/packages/happy-dom/package.json @@ -67,8 +67,8 @@ "scripts": { "compile": "npm run compile:esm && npm run compile:cjs npm run build-version-file", "compile:esm": "tsc", - "compile:cjs": "rm -rf ./cjs && tsc --moduleResolution Node --module CommonJS --outDir cjs && npm run change-cjs-file-extension", - "change-cjs-file-extension": "node ./bin/change-file-extension.cjs --dir=./cjs --fromExt=.js --toExt=.cjs", + "compile:cjs": "rm -rf ./cjs && tsc --moduleResolution Node --module CommonJS --outDir cjs && npm run compile:change-cjs-file-extension", + "compile:change-cjs-file-extension": "node ./bin/change-file-extension.cjs --dir=./cjs --fromExt=.js --toExt=.cjs", "build-version-file": "node ./bin/build-version-file.cjs", "watch": "tsc -w --preserveWatchOutput", "test": "vitest run --singleThread", diff --git a/packages/happy-dom/src/window/GlobalWindow.ts b/packages/happy-dom/src/window/GlobalWindow.ts index 3b2315b51..e641bd25a 100644 --- a/packages/happy-dom/src/window/GlobalWindow.ts +++ b/packages/happy-dom/src/window/GlobalWindow.ts @@ -1,4 +1,6 @@ import * as PropertySymbol from '../PropertySymbol.js'; +import { IOptionalBrowserSettings } from '../index.js'; +import BrowserWindow from './BrowserWindow.js'; import Window from './Window.js'; import { Buffer } from 'buffer'; @@ -70,6 +72,61 @@ export default class GlobalWindow extends Window { public gc: () => void = globalThis.gc; public v8debug?: unknown = globalThis.v8debug; + /** + * Constructor. + * + * @param [options] Options. + * @param [options.width] Window width. Defaults to "1024". + * @param [options.height] Window height. Defaults to "768". + * @param [options.innerWidth] Inner width. Deprecated. Defaults to "1024". + * @param [options.innerHeight] Inner height. Deprecated. Defaults to "768". + * @param [options.url] URL. + * @param [options.console] Console. + * @param [options.settings] Settings. + */ + constructor(options?: { + width?: number; + height?: number; + /** @deprecated Replaced by the "width" property. */ + innerWidth?: number; + /** @deprecated Replaced by the "height" property. */ + innerHeight?: number; + url?: string; + console?: Console; + settings?: IOptionalBrowserSettings; + }) { + super(options); + + /** + * Binds getts and setters, so that they will appear as an "own" property when using Object.getOwnPropertyNames(). + * + * This is needed for Vitest to work as it relies on Object.getOwnPropertyNames() to get the list of properties. + * + * @see https://github.com/capricorn86/happy-dom/issues/1339 + */ + for (const windowClass of [GlobalWindow, Window, BrowserWindow]) { + const propertyDescriptors = Object.getOwnPropertyDescriptors( + Reflect.getPrototypeOf(windowClass.prototype) + ); + + for (const key of Object.keys(propertyDescriptors)) { + const windowPropertyDescriptor = propertyDescriptors[key]; + if (windowPropertyDescriptor.get || windowPropertyDescriptor.set) { + const ownPropertyDescriptor = Object.getOwnPropertyDescriptor(this, key); + + if (!ownPropertyDescriptor) { + Object.defineProperty(this, key, { + configurable: true, + enumerable: windowPropertyDescriptor.enumerable, + get: windowPropertyDescriptor.get?.bind(this), + set: windowPropertyDescriptor.set?.bind(this) + }); + } + } + } + } + } + /** * Setup of VM context. */ diff --git a/packages/happy-dom/test/window/GlobalWindow.test.ts b/packages/happy-dom/test/window/GlobalWindow.test.ts index c30733a00..530b5c810 100644 --- a/packages/happy-dom/test/window/GlobalWindow.test.ts +++ b/packages/happy-dom/test/window/GlobalWindow.test.ts @@ -74,4 +74,37 @@ describe('GlobalWindow', () => { delete globalThis['variable']; }); }); + + describe('Object.getOwnPropertyNames()', () => { + it('Returns property names for Vitest.', () => { + const expected = [ + 'location', + 'history', + 'navigator', + 'screen', + 'sessionStorage', + 'localStorage', + 'opener', + 'scrollX', + 'pageXOffset', + 'scrollY', + 'pageYOffset', + 'CSS', + 'innerWidth', + 'innerHeight', + 'outerWidth', + 'outerHeight', + 'devicePixelRatio' + ]; + const included: string[] = []; + const propertyNames = Object.getOwnPropertyNames(window); + for (const name of expected) { + if (propertyNames.includes(name)) { + included.push(name); + } + } + + expect(included).toEqual(expected); + }); + }); });