Skip to content

Commit

Permalink
fix: [#1351] Fixes bug where calling Storage.getItem() with a key tha…
Browse files Browse the repository at this point in the history
…t has the same name as one of its methods or properties, returned the method/property
  • Loading branch information
capricorn86 committed Mar 23, 2024
1 parent 44dd9ce commit 6cdc5ed
Show file tree
Hide file tree
Showing 19 changed files with 307 additions and 121 deletions.
2 changes: 1 addition & 1 deletion packages/happy-dom/src/browser/Browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export default class Browser implements IBrowser {
*/
constructor(options?: { settings?: IOptionalBrowserSettings; console?: Console }) {
this.console = options?.console || null;
this.settings = BrowserSettingsFactory.getSettings(options?.settings);
this.settings = BrowserSettingsFactory.createSettings(options?.settings);
this.contexts = [new BrowserContext(this)];
}

Expand Down
2 changes: 1 addition & 1 deletion packages/happy-dom/src/browser/BrowserSettingsFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export default class BrowserSettingsFactory {
* @param [freezeObject] "true" to freeze the object.
* @returns Settings.
*/
public static getSettings(settings?: IOptionalBrowserSettings): IBrowserSettings {
public static createSettings(settings?: IOptionalBrowserSettings): IBrowserSettings {
return {
...DefaultBrowserSettings,
...settings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export default class DetachedBrowser implements IBrowser {
) {
this.windowClass = windowClass;
this.console = options?.console || null;
this.settings = BrowserSettingsFactory.getSettings(options?.settings);
this.settings = BrowserSettingsFactory.createSettings(options?.settings);
this.contexts = [];
this.contexts.push(new DetachedBrowserContext(this));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export default class BrowserFrameFactory {
* @param parentFrame Parent frame.
* @returns Frame.
*/
public static newChildFrame(parentFrame: IBrowserFrame): IBrowserFrame {
public static createChildFrame(parentFrame: IBrowserFrame): IBrowserFrame {
const frame = new (<new (page: IBrowserPage) => IBrowserFrame>parentFrame.constructor)(
parentFrame.page
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,61 +1,58 @@
import Element from './Element.js';
import * as PropertySymbol from '../../PropertySymbol.js';
import HTMLElementNamedNodeMap from '../html-element/HTMLElementNamedNodeMap.js';
import DatasetUtility from './DatasetUtility.js';
import IDataset from './IDataset.js';

/**
* Storage type for a dataset proxy.
*/
type DatasetRecord = Record<string, string>;

/**
* Dataset helper proxy.
* Dataset factory.
*
* Reference:
* https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/dataset
*/
export default class Dataset {
public readonly proxy: DatasetRecord;

export default class DatasetFactory {
/**
* @param element The parent element.
*/
constructor(element: Element) {
public static createDataset(element: Element): IDataset {
// Build the initial dataset record from all data attributes.
const dataset: DatasetRecord = {};
const dataset: IDataset = {};

for (let i = 0, max = element[PropertySymbol.attributes].length; i < max; i++) {
const attribute = element[PropertySymbol.attributes][i];
if (attribute[PropertySymbol.name].startsWith('data-')) {
const key = Dataset.kebabToCamelCase(attribute[PropertySymbol.name].replace('data-', ''));
const key = DatasetUtility.kebabToCamelCase(
attribute[PropertySymbol.name].replace('data-', '')
);
dataset[key] = attribute[PropertySymbol.value];
}
}

// Documentation for Proxy:
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy
this.proxy = new Proxy(dataset, {
get(dataset: DatasetRecord, key: string): string {
return new Proxy(dataset, {
get(dataset: IDataset, key: string): string {
const attribute = element[PropertySymbol.attributes].getNamedItem(
'data-' + Dataset.camelCaseToKebab(key)
'data-' + DatasetUtility.camelCaseToKebab(key)
);
if (attribute) {
return (dataset[key] = attribute[PropertySymbol.value]);
}
delete dataset[key];
return undefined;
},
set(dataset: DatasetRecord, key: string, value: string): boolean {
element.setAttribute('data-' + Dataset.camelCaseToKebab(key), value);
set(dataset: IDataset, key: string, value: string): boolean {
element.setAttribute('data-' + DatasetUtility.camelCaseToKebab(key), value);
dataset[key] = value;
return true;
},
deleteProperty(dataset: DatasetRecord, key: string): boolean {
deleteProperty(dataset: IDataset, key: string): boolean {
(<HTMLElementNamedNodeMap>element[PropertySymbol.attributes])[
PropertySymbol.removeNamedItem
]('data-' + Dataset.camelCaseToKebab(key));
]('data-' + DatasetUtility.camelCaseToKebab(key));
return delete dataset[key];
},
ownKeys(dataset: DatasetRecord): string[] {
ownKeys(dataset: IDataset): string[] {
// According to Mozilla we have to update the dataset object (target) to contain the same keys as what we return:
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy/Proxy/ownKeys
// "The result List must contain the keys of all non-configurable own properties of the target object."
Expand All @@ -64,7 +61,7 @@ export default class Dataset {
for (let i = 0, max = element[PropertySymbol.attributes].length; i < max; i++) {
const attribute = element[PropertySymbol.attributes][i];
if (attribute[PropertySymbol.name].startsWith('data-')) {
const key = Dataset.kebabToCamelCase(
const key = DatasetUtility.kebabToCamelCase(
attribute[PropertySymbol.name].replace('data-', '')
);
keys.push(key);
Expand All @@ -79,37 +76,11 @@ export default class Dataset {
}
return keys;
},
has(_dataset: DatasetRecord, key: string): boolean {
has(_dataset: IDataset, key: string): boolean {
return !!element[PropertySymbol.attributes].getNamedItem(
'data-' + Dataset.camelCaseToKebab(key)
'data-' + DatasetUtility.camelCaseToKebab(key)
);
}
});
}

/**
* Transforms a kebab cased string to camel case.
*
* @param text Text string.
* @returns Camel cased string.
*/
public static kebabToCamelCase(text: string): string {
const parts = text.split('-');
for (let i = 0, max = parts.length; i < max; i++) {
parts[i] = i > 0 ? parts[i].charAt(0).toUpperCase() + parts[i].slice(1) : parts[i];
}
return parts.join('');
}

/**
* Transforms a camel cased string to kebab case.
*
* @param text Text string.
* @returns Kebab cased string.
*/
public static camelCaseToKebab(text: string): string {
return text
.toString()
.replace(/[A-Z]+(?![a-z])|[A-Z]/g, ($, ofs) => (ofs ? '-' : '') + $.toLowerCase());
}
}
33 changes: 33 additions & 0 deletions packages/happy-dom/src/nodes/element/DatasetUtility.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* Dataset utility.
*
* Reference:
* https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/dataset
*/
export default class DatasetUtility {
/**
* Transforms a kebab cased string to camel case.
*
* @param text Text string.
* @returns Camel cased string.
*/
public static kebabToCamelCase(text: string): string {
const parts = text.split('-');
for (let i = 0, max = parts.length; i < max; i++) {
parts[i] = i > 0 ? parts[i].charAt(0).toUpperCase() + parts[i].slice(1) : parts[i];
}
return parts.join('');
}

/**
* Transforms a camel cased string to kebab case.
*
* @param text Text string.
* @returns Kebab cased string.
*/
public static camelCaseToKebab(text: string): string {
return text
.toString()
.replace(/[A-Z]+(?![a-z])|[A-Z]/g, ($, ofs) => (ofs ? '-' : '') + $.toLowerCase());
}
}
3 changes: 3 additions & 0 deletions packages/happy-dom/src/nodes/element/IDataset.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default interface IDataset {
[key: string]: string;
}
9 changes: 5 additions & 4 deletions packages/happy-dom/src/nodes/html-element/HTMLElement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import Element from '../element/Element.js';
import * as PropertySymbol from '../../PropertySymbol.js';
import CSSStyleDeclaration from '../../css/declaration/CSSStyleDeclaration.js';
import PointerEvent from '../../event/events/PointerEvent.js';
import Dataset from '../element/Dataset.js';
import NodeTypeEnum from '../node/NodeTypeEnum.js';
import DOMException from '../../exception/DOMException.js';
import Event from '../../event/Event.js';
Expand All @@ -12,6 +11,8 @@ import HTMLElementNamedNodeMap from './HTMLElementNamedNodeMap.js';
import NodeList from '../node/NodeList.js';
import Node from '../node/Node.js';
import HTMLCollection from '../element/HTMLCollection.js';
import DatasetFactory from '../element/DatasetFactory.js';
import IDataset from '../element/IDataset.js';

/**
* HTML Element.
Expand Down Expand Up @@ -63,7 +64,7 @@ export default class HTMLElement extends Element {
public [PropertySymbol.style]: CSSStyleDeclaration = null;

// Private properties
#dataset: Dataset = null;
#dataset: IDataset = null;
#customElementDefineCallback: () => void = null;

/**
Expand Down Expand Up @@ -353,8 +354,8 @@ export default class HTMLElement extends Element {
*
* @returns Data set.
*/
public get dataset(): { [key: string]: string } {
return (this.#dataset ??= new Dataset(this)).proxy;
public get dataset(): IDataset {
return (this.#dataset ??= DatasetFactory.createDataset(this));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export default class HTMLIFrameElementPageLoader {
const parentWindow = isSameOrigin ? window : new CrossOriginBrowserWindow(window);

this.#browserIFrame =
this.#browserIFrame ?? BrowserFrameFactory.newChildFrame(this.#browserParentFrame);
this.#browserIFrame ?? BrowserFrameFactory.createChildFrame(this.#browserParentFrame);

(<BrowserWindow | CrossOriginBrowserWindow>(<unknown>this.#browserIFrame.window.top)) =
parentWindow;
Expand Down
9 changes: 5 additions & 4 deletions packages/happy-dom/src/nodes/svg-element/SVGElement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ import * as PropertySymbol from '../../PropertySymbol.js';
import Element from '../element/Element.js';
import SVGSVGElement from './SVGSVGElement.js';
import Event from '../../event/Event.js';
import Dataset from '../element/Dataset.js';
import HTMLElementUtility from '../html-element/HTMLElementUtility.js';
import NamedNodeMap from '../../named-node-map/NamedNodeMap.js';
import SVGElementNamedNodeMap from './SVGElementNamedNodeMap.js';
import DatasetFactory from '../element/DatasetFactory.js';
import IDataset from '../element/IDataset.js';

/**
* SVG Element.
Expand All @@ -28,7 +29,7 @@ export default class SVGElement extends Element {
public [PropertySymbol.style]: CSSStyleDeclaration | null = null;

// Private properties
#dataset: Dataset = null;
#dataset: IDataset = null;

/**
* Returns viewport.
Expand Down Expand Up @@ -61,8 +62,8 @@ export default class SVGElement extends Element {
*
* @returns Data set.
*/
public get dataset(): { [key: string]: string } {
return (this.#dataset ??= new Dataset(this)).proxy;
public get dataset(): IDataset {
return (this.#dataset ??= DatasetFactory.createDataset(this));
}

/**
Expand Down
68 changes: 56 additions & 12 deletions packages/happy-dom/src/storage/Storage.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,63 @@
import * as PropertySymbol from '../PropertySymbol.js';

/**
* Storage.
*
* @see https://developer.mozilla.org/en-US/docs/Web/API/Storage
*/
export default class Storage {
public [PropertySymbol.data]: { [key: string]: string } = {};

/**
*
*/
constructor() {
const descriptors = Object.getOwnPropertyDescriptors(Storage.prototype);

Object.defineProperty(this, 'length', {
enumerable: false,
configurable: true,
get: descriptors['length'].get.bind(this)
});

Object.defineProperty(this, 'key', {
enumerable: false,
configurable: true,
value: descriptors['key'].value.bind(this)
});

Object.defineProperty(this, 'setItem', {
enumerable: false,
configurable: true,
value: descriptors['setItem'].value.bind(this)
});

Object.defineProperty(this, 'getItem', {
enumerable: false,
configurable: true,
value: descriptors['getItem'].value.bind(this)
});

Object.defineProperty(this, 'removeItem', {
enumerable: false,
configurable: true,
value: descriptors['removeItem'].value.bind(this)
});

Object.defineProperty(this, 'clear', {
enumerable: false,
configurable: true,
value: descriptors['clear'].value.bind(this)
});
}

/**
* Returns length.
*
* @returns Length.
*/
public get length(): number {
return Object.keys(this).length;
return Object.keys(this[PropertySymbol.data]).length;
}

/**
Expand All @@ -19,9 +66,9 @@ export default class Storage {
* @param index Index.
* @returns Name.
*/
public key(index: number): string {
const name = Object.keys(this)[index];
return name === undefined ? null : name;
public key(index: number): string | null {
const name = Object.keys(this[PropertySymbol.data])[index];
return name !== undefined ? name : null;
}

/**
Expand All @@ -31,7 +78,7 @@ export default class Storage {
* @param item Item.
*/
public setItem(name: string, item: string): void {
this[name] = String(item);
this[PropertySymbol.data][name] = String(item);
}

/**
Expand All @@ -40,8 +87,8 @@ export default class Storage {
* @param name Name.
* @returns Item.
*/
public getItem(name: string): string {
return this[name] === undefined ? null : this[name];
public getItem(name: string): string | null {
return this[PropertySymbol.data][name] !== undefined ? this[PropertySymbol.data][name] : null;
}

/**
Expand All @@ -50,16 +97,13 @@ export default class Storage {
* @param name Name.
*/
public removeItem(name: string): void {
delete this[name];
delete this[PropertySymbol.data][name];
}

/**
* Clears storage.
*/
public clear(): void {
const keys = Object.keys(this);
for (const key of keys) {
delete this[key];
}
this[PropertySymbol.data] = {};
}
}
Loading

0 comments on commit 6cdc5ed

Please sign in to comment.