Skip to content

Commit

Permalink
Merge pull request #1038 from RussianCow/task/652-fix-dataset-delete-…
Browse files Browse the repository at this point in the history
…nonexistent-key

#652@patch: Allow deletion of nonexistent keys from dataset
  • Loading branch information
capricorn86 committed Sep 25, 2023
2 parents bcb0225 + b1118ed commit 75efbdc
Show file tree
Hide file tree
Showing 15 changed files with 73 additions and 29 deletions.
26 changes: 23 additions & 3 deletions 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.
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions 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.
Expand Down Expand Up @@ -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:
Expand Down
6 changes: 5 additions & 1 deletion packages/happy-dom/src/nodes/element/Element.ts
Expand Up @@ -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.
}
}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/happy-dom/src/nodes/element/ElementNamedNodeMap.ts
Expand Up @@ -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;
Expand Down
Expand Up @@ -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) {
Expand Down
Expand Up @@ -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 &&
Expand Down
Expand Up @@ -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 = '';
Expand Down
Expand Up @@ -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 &&
Expand Down
Expand Up @@ -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();
Expand Down
Expand Up @@ -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;
Expand Down
Expand Up @@ -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 &&
Expand Down
Expand Up @@ -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 &&
Expand Down
Expand Up @@ -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 = '';
Expand Down
21 changes: 19 additions & 2 deletions packages/happy-dom/test/named-node-map/NamedNodeMap.test.ts
Expand Up @@ -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', () => {
Expand All @@ -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]');
});
});
Expand Down Expand Up @@ -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
)
);
});
});
});
Expand Up @@ -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
Expand Down

0 comments on commit 75efbdc

Please sign in to comment.