Merge pull request #1344 from capricorn86/1343-fix-own-properties-for…

…-getters-in-window-class fix: [#1343] Bingd getters and setters on all Window classes (not onl…
capricorn86 · Mar 21, 2024 · 2ce84c0 · 2ce84c0
2 parents f42adfa + a8a0616
commit 2ce84c0
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 61 deletions.
diff --git a/packages/happy-dom/src/window/BrowserWindow.ts b/packages/happy-dom/src/window/BrowserWindow.ts
@@ -530,11 +530,24 @@ export default class BrowserWindow extends EventTarget implements INodeJSGlobal
 
 		WindowBrowserSettingsReader.setSettings(this, this.#browserFrame.page.context.browser.settings);
 
+		// 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
 		// Binds all methods to "this", so that it will use the correct context when called globally.
-		for (const key of Object.getOwnPropertyNames(BrowserWindow.prototype).concat(
-			Object.getOwnPropertyNames(EventTarget.prototype)
-		)) {
-			if (
+		const propertyDescriptors = Object.assign(
+			Object.getOwnPropertyDescriptors(EventTarget.prototype),
+			Object.getOwnPropertyDescriptors(BrowserWindow.prototype)
+		);
+		for (const key of Object.keys(propertyDescriptors)) {
+			const descriptor = propertyDescriptors[key];
+			if (descriptor.get || descriptor.set) {
+				Object.defineProperty(this, key, {
+					configurable: true,
+					enumerable: true,
+					get: descriptor.get?.bind(this),
+					set: descriptor.set?.bind(this)
+				});
+			} else if (
 				key !== 'constructor' &&
 				key[0] !== '_' &&
 				key[0] === key[0].toLowerCase() &&

diff --git a/packages/happy-dom/src/window/GlobalWindow.ts b/packages/happy-dom/src/window/GlobalWindow.ts
@@ -1,6 +1,4 @@
 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';
 
@@ -72,61 +70,6 @@ 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/BrowserWindow.test.ts b/packages/happy-dom/test/window/BrowserWindow.test.ts
@@ -1640,4 +1640,36 @@ describe('BrowserWindow', () => {
 			expect(newWindow.outerHeight).toBe(768 - 200);
 		});
 	});
+	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);
+		});
+	});
 });
diff --git a/packages/happy-dom/test/window/Window.test.ts b/packages/happy-dom/test/window/Window.test.ts
@@ -342,4 +342,36 @@ describe('Window', () => {
 			expect(newWindow2.document.body.innerHTML).toBe('Test');
 		});
 	});
+	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);
+		});
+	});
 });