Merge pull request #1038 from RussianCow/task/652-fix-dataset-delete-…

…nonexistent-key #652@patch: Allow deletion of nonexistent keys from dataset
capricorn86 · Sep 25, 2023 · 75efbdc · 75efbdc
2 parents bcb0225 + b1118ed
commit 75efbdc
Show file tree

Hide file tree

Showing 15 changed files with 73 additions and 29 deletions.
diff --git a/packages/happy-dom/src/named-node-map/NamedNodeMap.ts b/packages/happy-dom/src/named-node-map/NamedNodeMap.ts
@@ -1,5 +1,7 @@
 import INamedNodeMap from './INamedNodeMap.js';
 import IAttr from '../nodes/attr/IAttr.js';
+import DOMException from '../exception/DOMException.js';
+import DOMExceptionNameEnum from '../exception/DOMExceptionNameEnum.js';
 
 /**
  * Named Node Map.
@@ -97,11 +99,19 @@ export default class NamedNodeMap implements INamedNodeMap {
 	/**
 	 * Removes an item.
 	 *
+	 * @throws DOMException
 	 * @param name Name of item.
 	 * @returns Removed item.
 	 */
-	public removeNamedItem(name: string): IAttr | null {
-		return this._removeNamedItemWithoutConsequences(name);
+	public removeNamedItem(name: string): IAttr {
+		const item = this._removeNamedItem(name);
+		if (!item) {
+			throw new DOMException(
+				`Failed to execute 'removeNamedItem' on 'NamedNodeMap': No item with name '${name}' was found.`,
+				DOMExceptionNameEnum.notFoundError
+			);
+		}
+		return item;
 	}
 
 	/**
@@ -147,11 +157,21 @@ export default class NamedNodeMap implements INamedNodeMap {
 		return null;
 	}
 
+	/**
+	 * Removes an item without throwing if it doesn't exist.
+	 *
+	 * @param name Name of item.
+	 * @returns Removed item, or null if it didn't exist.
+	 */
+	public _removeNamedItem(name: string): IAttr | null {
+		return this._removeNamedItemWithoutConsequences(name);
+	}
+
 	/**
 	 * Removes an item without calling listeners for certain attributes.
 	 *
 	 * @param name Name of item.
-	 * @returns Removed item.
+	 * @returns Removed item, or null if it didn't exist.
 	 */
 	public _removeNamedItemWithoutConsequences(name: string): IAttr | null {
 		const removedItem = this._namedItems[name] || null;

diff --git a/packages/happy-dom/src/nodes/element/Dataset.ts b/packages/happy-dom/src/nodes/element/Dataset.ts
@@ -1,4 +1,5 @@
 import Element from '../element/Element.js';
+import HTMLElementNamedNodeMap from '../html-element/HTMLElementNamedNodeMap.js';
 
 /**
  * Storage type for a dataset proxy.
@@ -46,10 +47,10 @@ export default class Dataset {
 				return true;
 			},
 			deleteProperty(dataset: DatasetRecord, key: string): boolean {
-				return (
-					!!element.attributes.removeNamedItem('data-' + Dataset.camelCaseToKebab(key)) &&
-					delete dataset[key]
+				(<HTMLElementNamedNodeMap>element.attributes)._removeNamedItem(
+					'data-' + Dataset.camelCaseToKebab(key)
 				);
+				return delete dataset[key];
 			},
 			ownKeys(dataset: DatasetRecord): string[] {
 				// According to Mozilla we have to update the dataset object (target) to contain the same keys as what we return:

diff --git a/packages/happy-dom/src/nodes/element/Element.ts b/packages/happy-dom/src/nodes/element/Element.ts
@@ -658,7 +658,11 @@ export default class Element extends Node implements IElement {
 	 * @param name Name.
 	 */
 	public removeAttribute(name: string): void {
-		this.attributes.removeNamedItem(name);
+		try {
+			this.attributes.removeNamedItem(name);
+		} catch (error) {
+			// Ignore DOMException when the attribute does not exist.
+		}
 	}
 
 	/**

diff --git a/packages/happy-dom/src/nodes/element/ElementNamedNodeMap.ts b/packages/happy-dom/src/nodes/element/ElementNamedNodeMap.ts
@@ -114,8 +114,8 @@ export default class ElementNamedNodeMap extends NamedNodeMap {
 	/**
 	 * @override
 	 */
-	public override removeNamedItem(name: string): IAttr | null {
-		const removedItem = super.removeNamedItem(this._getAttributeName(name));
+	public override _removeNamedItem(name: string): IAttr | null {
+		const removedItem = super._removeNamedItem(this._getAttributeName(name));
 
 		if (!removedItem) {
 			return null;

diff --git a/packages/happy-dom/src/nodes/html-anchor-element/HTMLAnchorElementNamedNodeMap.ts b/packages/happy-dom/src/nodes/html-anchor-element/HTMLAnchorElementNamedNodeMap.ts
@@ -32,8 +32,8 @@ export default class HTMLAnchorElementNamedNodeMap extends HTMLElementNamedNodeM
 	/**
 	 * @override
 	 */
-	public override removeNamedItem(name: string): IAttr | null {
-		const removedItem = super.removeNamedItem(name);
+	public override _removeNamedItem(name: string): IAttr | null {
+		const removedItem = super._removeNamedItem(name);
 
 		if (removedItem) {
 			if (removedItem.name === 'rel' && this._ownerElement._relList) {

diff --git a/packages/happy-dom/src/nodes/html-button-element/HTMLButtonElementNamedNodeMap.ts b/packages/happy-dom/src/nodes/html-button-element/HTMLButtonElementNamedNodeMap.ts
@@ -38,8 +38,8 @@ export default class HTMLButtonElementNamedNodeMap extends HTMLElementNamedNodeM
 	/**
 	 * @override
 	 */
-	public override removeNamedItem(name: string): IAttr | null {
-		const removedItem = super.removeNamedItem(name);
+	public override _removeNamedItem(name: string): IAttr | null {
+		const removedItem = super._removeNamedItem(name);
 
 		if (
 			removedItem &&

diff --git a/packages/happy-dom/src/nodes/html-element/HTMLElementNamedNodeMap.ts b/packages/happy-dom/src/nodes/html-element/HTMLElementNamedNodeMap.ts
@@ -26,8 +26,8 @@ export default class HTMLElementNamedNodeMap extends ElementNamedNodeMap {
 	/**
 	 * @override
 	 */
-	public override removeNamedItem(name: string): IAttr | null {
-		const removedItem = super.removeNamedItem(name);
+	public override _removeNamedItem(name: string): IAttr | null {
+		const removedItem = super._removeNamedItem(name);
 
 		if (removedItem && removedItem.name === 'style' && this._ownerElement._style) {
 			this._ownerElement._style.cssText = '';

diff --git a/packages/happy-dom/src/nodes/html-input-element/HTMLInputElementNamedNodeMap.ts b/packages/happy-dom/src/nodes/html-input-element/HTMLInputElementNamedNodeMap.ts
@@ -38,8 +38,8 @@ export default class HTMLInputElementNamedNodeMap extends HTMLElementNamedNodeMa
 	/**
 	 * @override
 	 */
-	public override removeNamedItem(name: string): IAttr | null {
-		const removedItem = super.removeNamedItem(name);
+	public override _removeNamedItem(name: string): IAttr | null {
+		const removedItem = super._removeNamedItem(name);
 
 		if (
 			removedItem &&

diff --git a/packages/happy-dom/src/nodes/html-link-element/HTMLLinkElementNamedNodeMap.ts b/packages/happy-dom/src/nodes/html-link-element/HTMLLinkElementNamedNodeMap.ts
@@ -31,8 +31,8 @@ export default class HTMLLinkElementNamedNodeMap extends HTMLElementNamedNodeMap
 	/**
 	 * @override
 	 */
-	public override removeNamedItem(name: string): IAttr | null {
-		const removedItem = super.removeNamedItem(name);
+	public override _removeNamedItem(name: string): IAttr | null {
+		const removedItem = super._removeNamedItem(name);
 
 		if (removedItem && removedItem.name === 'rel' && this._ownerElement._relList) {
 			this._ownerElement._relList._updateIndices();

diff --git a/packages/happy-dom/src/nodes/html-option-element/HTMLOptionElementNamedNodeMap.ts b/packages/happy-dom/src/nodes/html-option-element/HTMLOptionElementNamedNodeMap.ts
@@ -37,8 +37,8 @@ export default class HTMLOptionElementNamedNodeMap extends HTMLElementNamedNodeM
 	/**
 	 * @override
 	 */
-	public override removeNamedItem(name: string): IAttr | null {
-		const removedItem = super.removeNamedItem(name);
+	public override _removeNamedItem(name: string): IAttr | null {
+		const removedItem = super._removeNamedItem(name);
 
 		if (removedItem && !this._ownerElement._dirtyness && removedItem.name === 'selected') {
 			const selectNode = <HTMLSelectElement>this._ownerElement._selectNode;

diff --git a/packages/happy-dom/src/nodes/html-select-element/HTMLSelectElementNamedNodeMap.ts b/packages/happy-dom/src/nodes/html-select-element/HTMLSelectElementNamedNodeMap.ts
@@ -38,8 +38,8 @@ export default class HTMLSelectElementNamedNodeMap extends HTMLElementNamedNodeM
 	/**
 	 * @override
 	 */
-	public override removeNamedItem(name: string): IAttr | null {
-		const removedItem = super.removeNamedItem(name);
+	public override _removeNamedItem(name: string): IAttr | null {
+		const removedItem = super._removeNamedItem(name);
 
 		if (
 			removedItem &&

diff --git a/packages/happy-dom/src/nodes/html-text-area-element/HTMLTextAreaElementNamedNodeMap.ts b/packages/happy-dom/src/nodes/html-text-area-element/HTMLTextAreaElementNamedNodeMap.ts
@@ -38,8 +38,8 @@ export default class HTMLTextAreaElementNamedNodeMap extends HTMLElementNamedNod
 	/**
 	 * @override
 	 */
-	public override removeNamedItem(name: string): IAttr | null {
-		const removedItem = super.removeNamedItem(name);
+	public override _removeNamedItem(name: string): IAttr | null {
+		const removedItem = super._removeNamedItem(name);
 
 		if (
 			removedItem &&

diff --git a/packages/happy-dom/src/nodes/svg-element/SVGElementNamedNodeMap.ts b/packages/happy-dom/src/nodes/svg-element/SVGElementNamedNodeMap.ts
@@ -26,8 +26,8 @@ export default class SVGElementNamedNodeMap extends ElementNamedNodeMap {
 	/**
 	 * @override
 	 */
-	public override removeNamedItem(name: string): IAttr | null {
-		const removedItem = super.removeNamedItem(name);
+	public override _removeNamedItem(name: string): IAttr | null {
+		const removedItem = super._removeNamedItem(name);
 
 		if (removedItem && removedItem.name === 'style' && this._ownerElement._style) {
 			this._ownerElement._style.cssText = '';

diff --git a/packages/happy-dom/test/named-node-map/NamedNodeMap.test.ts b/packages/happy-dom/test/named-node-map/NamedNodeMap.test.ts
@@ -4,6 +4,8 @@ import IDocument from '../../src/nodes/document/IDocument.js';
 import IElement from '../../src/nodes/element/IElement.js';
 import INamedNodeMap from '../../src/named-node-map/INamedNodeMap.js';
 import IAttr from '../../src/nodes/attr/IAttr.js';
+import DOMException from '../../src/exception/DOMException.js';
+import DOMExceptionNameEnum from '../../src/exception/DOMExceptionNameEnum.js';
 import { beforeEach, describe, it, expect } from 'vitest';
 
 describe('NamedNodeMap', () => {
@@ -20,13 +22,13 @@ describe('NamedNodeMap', () => {
 	});
 
 	describe('get toString()', () => {
-		it('Returns a stirng.', () => {
+		it('Returns a string.', () => {
 			expect(attributes.toString()).toBe('[object NamedNodeMap]');
 		});
 	});
 
 	describe('get toString()', () => {
-		it('Returns a stirng.', () => {
+		it('Returns a string.', () => {
 			expect(attributes.toString()).toBe('[object NamedNodeMap]');
 		});
 	});
@@ -156,5 +158,20 @@ describe('NamedNodeMap', () => {
 
 			expect(element.getAttribute('key')).toBe(null);
 		});
+
+		it('Throws a NotFoundError on a missing attribute.', () => {
+			let error: Error | null = null;
+			try {
+				attributes.removeNamedItem('non-existent-attribute');
+			} catch (e) {
+				error = e;
+			}
+			expect(error).toEqual(
+				new DOMException(
+					"Failed to execute 'removeNamedItem' on 'NamedNodeMap': No item with name 'non-existent-attribute' was found.",
+					DOMExceptionNameEnum.notFoundError
+				)
+			);
+		});
 	});
 });
diff --git a/packages/happy-dom/test/nodes/html-element/HTMLElement.test.ts b/packages/happy-dom/test/nodes/html-element/HTMLElement.test.ts
@@ -300,6 +300,8 @@ describe('HTMLElement', () => {
 			expect(element.getAttribute('data-test-delta')).toBe(null);
 			expect(Object.keys(dataset)).toEqual(['testAlpha', 'testBeta', 'testGamma']);
 			expect(Object.values(dataset)).toEqual(['value2', 'value4', 'value5']);
+
+			delete dataset.nonExistentKey;
 		});
 
 		// https://github.com/capricorn86/happy-dom/issues/493