From 3070ab77c98d99b021a70a419087809475d38df9 Mon Sep 17 00:00:00 2001 From: b-fuze Date: Sat, 23 Nov 2024 07:35:45 -0500 Subject: [PATCH 1/6] chore: WIP memory optimizations --- src/deserialize.ts | 6 +- src/dom/document.ts | 21 +- src/dom/element.ts | 455 +++++++++++++++++----- src/dom/elements/html-template-element.ts | 6 +- src/dom/html-collection.ts | 4 +- src/dom/node-list.ts | 4 +- src/dom/node.ts | 96 +++-- src/dom/string-cache.ts | 12 + src/dom/utils.ts | 5 +- 9 files changed, 467 insertions(+), 142 deletions(-) create mode 100644 src/dom/string-cache.ts diff --git a/src/deserialize.ts b/src/deserialize.ts index 1a1ec33..10970f2 100644 --- a/src/deserialize.ts +++ b/src/deserialize.ts @@ -56,13 +56,13 @@ function nodeFromArray(data: any, parentNode: Node | null): Node { switch (child[0]) { case NodeType.TEXT_NODE: childNode = new Text(child[1]); - childNode.parentNode = childNode.parentElement = elm; + childNode.parentNode = elm; childNodes.push(childNode); break; case NodeType.COMMENT_NODE: childNode = new Comment(child[1]); - childNode.parentNode = childNode.parentElement = elm; + childNode.parentNode = elm; childNodes.push(childNode); break; @@ -73,7 +73,7 @@ function nodeFromArray(data: any, parentNode: Node | null): Node { case NodeType.DOCUMENT_TYPE_NODE: childNode = new DocumentType(child[1], child[2], child[3], CTOR_KEY); - childNode.parentNode = childNode.parentElement = elm; + childNode.parentNode = elm; childNodes.push(childNode); break; } diff --git a/src/dom/document.ts b/src/dom/document.ts index 08d4efb..bac955c 100644 --- a/src/dom/document.ts +++ b/src/dom/document.ts @@ -7,6 +7,7 @@ import { HTMLTemplateElement } from "./elements/html-template-element.ts"; import { getSelectorEngine, SelectorApi } from "./selectors/selectors.ts"; import { getElementsByClassName } from "./utils.ts"; import UtilTypes from "./utils-types.ts"; +import { getUpperCase } from "./string-cache.ts"; export class DOMImplementation { constructor(key: typeof CTOR_KEY) { @@ -94,7 +95,7 @@ export class DocumentType extends Node { return this.#systemId; } - _shallowClone(): Node { + override _shallowClone(): Node { return new DocumentType( this.#qualifiedName, this.#publicId, @@ -119,7 +120,7 @@ export class Document extends Node { public body: Element = null; public implementation: DOMImplementation; - #lockState = false; + // #lockState = false; #documentURI = "about:blank"; // TODO #title = ""; #nwapi: SelectorApi | null = null; @@ -135,7 +136,7 @@ export class Document extends Node { this.implementation = new DOMImplementation(CTOR_KEY); } - _shallowClone(): Node { + override _shallowClone(): Node { return new Document(); } @@ -215,14 +216,14 @@ export class Document extends Node { return count; } - appendChild(child: Node): Node { + override appendChild(child: Node): Node { super.appendChild(child); child._setOwnerDocument(this); return child; } createElement(tagName: string, options?: ElementCreationOptions): Element { - tagName = tagName.toUpperCase(); + tagName = getUpperCase(tagName); switch (tagName) { case "TEMPLATE": { @@ -299,7 +300,7 @@ export class Document extends Node { // but that would be a breaking change since `.body` // and `.head` would need to be typed as `Element | null`. // Currently they're typed as `Element` which is incorrect... - cloneNode(deep?: boolean): Document { + override cloneNode(deep?: boolean): Document { const doc = super.cloneNode(deep) as Document; for (const child of doc.documentElement?.childNodes || []) { @@ -338,6 +339,10 @@ export class Document extends Node { // TODO: DRY!!! getElementById(id: string): Element | null { + if (!this._hasInitializedChildNodes()) { + return null; + } + for (const child of this.childNodes) { if (child.nodeType === NodeType.ELEMENT_NODE) { if (( child).id === id) { @@ -363,7 +368,7 @@ export class Document extends Node { ) : []; } else { - return this._getElementsByTagName(tagName.toUpperCase(), []); + return this._getElementsByTagName(getUpperCase(tagName), []); } } @@ -413,7 +418,7 @@ export class HTMLDocument extends Document { super(); } - _shallowClone(): Node { + override _shallowClone(): Node { return new HTMLDocument(CTOR_KEY); } } diff --git a/src/dom/element.ts b/src/dom/element.ts index 6a0e186..d85fc69 100644 --- a/src/dom/element.ts +++ b/src/dom/element.ts @@ -13,12 +13,17 @@ import { upperCaseCharRe, } from "./utils.ts"; import UtilTypes from "./utils-types.ts"; +import { getUpperCase, getLowerCase } from "./string-cache.ts"; export interface DOMTokenList { [index: number]: string; } export class DOMTokenList { + // Minimum number of classnames/tokens in order to switch from + // an array-backed to a set-backed list + static #DOM_TOKEN_LIST_MIN_SET_SIZE = 32 as const; + #_value = ""; get #value() { return this.#_value; @@ -29,7 +34,7 @@ export class DOMTokenList { this.#_value = value; this.#onChange(value); } - #set = new Set(); + #set: string[] | Set = []; #onChange: (className: string) => void; constructor( @@ -59,12 +64,23 @@ export class DOMTokenList { input: string, ) { this.#value = input; - this.#set = new Set( - input - .trim() - .split(/[\t\n\f\r\s]+/g) - .filter(Boolean), - ); + this.#set = input + .trim() + .split(/[\t\n\f\r\s]+/g) + .filter(Boolean); + + if (this.#set.length > DOMTokenList.#DOM_TOKEN_LIST_MIN_SET_SIZE) { + this.#set = new Set(this.#set); + } else { + const deduplicatedSet: string[] = []; + for (const element of this.#set) { + if (!deduplicatedSet.includes(element)) { + deduplicatedSet.push(element); + } + } + this.#set = deduplicatedSet; + } + this.#setIndices(); } @@ -73,7 +89,11 @@ export class DOMTokenList { } get length(): number { - return this.#set.size; + if (this.#set.constructor === Array) { + return this.#set.length; + } else { + return (this.#set as Set).size; + } } *entries(): IterableIterator<[number, string]> { @@ -88,7 +108,8 @@ export class DOMTokenList { } *keys(): IterableIterator { - for (let i = 0; i < this.#set.size; i++) { + const length = this.length; + for (let i = 0; i < length; i++) { yield i; } } @@ -108,41 +129,73 @@ export class DOMTokenList { contains( element: string, ): boolean { - return this.#set.has(element); + if (this.#set.constructor === Array) { + return this.#set.includes(element); + } else { + return (this.#set as Set).has(element); + } + } + + #arrayAdd(element: string) { + const array = this.#set as string[]; + if (!array.includes(element)) { + this[array.length] = element; + array.push(element); + } + } + #setAdd(element: string) { + const set = this.#set as Set; + const { size } = set; + set.add(element); + if (size < set.size) { + this[size] = element; + } } add( ...elements: Array ) { + const method = (this.#set.constructor === Array ? this.#arrayAdd : this.#setAdd).bind(this); + for (const element of elements) { if (DOMTokenList.#invalidToken(element)) { throw new DOMException( "Failed to execute 'add' on 'DOMTokenList': The token provided must not be empty.", ); } - const { size } = this.#set; - this.#set.add(element); - if (size < this.#set.size) { - this[size] = element; - } + + method(element); } this.#updateClassString(); } + #arrayRemove(element: string) { + const array = this.#set as string[]; + const index = array.indexOf(element); + if (index >= 0) { + array.splice(index, 1); + } + } + #setRemove(element: string) { + (this.#set as Set).delete(element); + } + remove( ...elements: Array ) { - const { size } = this.#set; + const method = (this.#set.constructor === Array ? this.#arrayRemove : this.#setRemove).bind(this); + const size = this.length; for (const element of elements) { if (DOMTokenList.#invalidToken(element)) { throw new DOMException( "Failed to execute 'remove' on 'DOMTokenList': The token provided must not be empty.", ); } - this.#set.delete(element); + method(element); } - if (size !== this.#set.size) { - for (let i = this.#set.size; i < size; i++) { + const newSize = this.length; + if (size !== newSize) { + for (let i = newSize; i < size; i++) { delete this[i]; } this.#setIndices(); @@ -154,20 +207,22 @@ export class DOMTokenList { oldToken: string, newToken: string, ): boolean { + const removeMethod = (this.#set.constructor === Array ? this.#arrayRemove : this.#setRemove).bind(this); + const addMethod = (this.#set.constructor === Array ? this.#arrayAdd : this.#setAdd).bind(this); if ([oldToken, newToken].some((v) => DOMTokenList.#invalidToken(v))) { throw new DOMException( "Failed to execute 'replace' on 'DOMTokenList': The token provided must not be empty.", ); } - if (!this.#set.has(oldToken)) { + if (!this.contains(oldToken)) { return false; } - if (this.#set.has(newToken)) { + if (this.contains(newToken)) { this.remove(oldToken); } else { - this.#set.delete(oldToken); - this.#set.add(newToken); + removeMethod(oldToken); + addMethod(newToken); this.#setIndices(); this.#updateClassString(); } @@ -204,9 +259,90 @@ export class DOMTokenList { #updateClassString() { this.#value = Array.from(this.#set).join(" "); + + if (this.#set.constructor === Array && this.#set.length > DOMTokenList.#DOM_TOKEN_LIST_MIN_SET_SIZE) { + throw new Error("how: " + this.#set.length + " " + this.#set.join(",")); + this.#set = new Set(this.#set); + } } } +const initializeClassListSym = Symbol("initializeClassListSym"); +const domTokenListCurrentElementSym = Symbol("domTokenListCurrentElementSym"); +class UinitializedDOMTokenList { + // This will always be populated with the current element + // being queried + + [domTokenListCurrentElementSym]: Element; + constructor( + currentElement: Element, + ) { + this[domTokenListCurrentElementSym] = currentElement; + } + + set value(input: string) { + if (input?.trim?.() !== "") { + this[domTokenListCurrentElementSym][initializeClassListSym](); + this[domTokenListCurrentElementSym].classList.value = input; + } + } + + get value(): string { + return ""; + } + + get length(): number { + return 0; + } + + *entries(): IterableIterator<[number, string]> {} + *values(): IterableIterator {} + *keys(): IterableIterator {} + + *[Symbol.iterator](): IterableIterator { + yield* this.values(); + } + + item(_index: number): string | null { + return null; + } + + contains(_element: string): boolean { + return false; + } + + add(...elements: string[]) { + this[domTokenListCurrentElementSym][initializeClassListSym](); + this[domTokenListCurrentElementSym].classList.add(...elements); + } + + remove(..._elements: string[]) {} + replace(_oldToken: string, _newToken: string) { + return false; + } + + supports(): never { + throw new Error("Not implemented"); + } + + toggle( + element: string, + force?: boolean, + ): boolean { + if (force === false) { + return false; + } + + this[domTokenListCurrentElementSym][initializeClassListSym](); + this[domTokenListCurrentElementSym].classList.add(element); + return true; + } + + forEach( + _callback: (value: string, index: number, list: DOMTokenList) => void, + ) {} +}; + const setNamedNodeMapOwnerElementSym = Symbol("setNamedNodeMapOwnerElementSym"); const setAttrValueSym = Symbol("setAttrValueSym"); export class Attr extends Node { @@ -321,6 +457,7 @@ const getNamedNodeMapValueSym = Symbol("getNamedNodeMapValueSym"); const getNamedNodeMapAttrNamesSym = Symbol("getNamedNodeMapAttrNamesSym"); const getNamedNodeMapAttrNodeSym = Symbol("getNamedNodeMapAttrNodeSym"); const removeNamedNodeMapAttrSym = Symbol("removeNamedNodeMapAttrSym"); +const getOwnerElementIdClassNameAttrOrderingSym = Symbol("getOwnerElementIdClassNameAttrOrderingSym"); export class NamedNodeMap { static #indexedAttrAccess = function ( this: NamedNodeMap, @@ -337,11 +474,11 @@ export class NamedNodeMap { ?.slice(1); // Remove "a" for safeAttrName return this[getNamedNodeMapAttrNodeSym](attribute); }; - #onAttrNodeChange: (attr: string, value: string | null) => void; + #onAttrNodeChange: (attr: string, value: string | null, isRemoved: boolean) => void; constructor( ownerElement: Element, - onAttrNodeChange: (attr: string, value: string | null) => void, + onAttrNodeChange: (attr: string, value: string | null, isRemoved: boolean) => void, key: typeof CTOR_KEY, ) { if (key !== CTOR_KEY) { @@ -349,6 +486,12 @@ export class NamedNodeMap { } this.#ownerElement = ownerElement; this.#onAttrNodeChange = onAttrNodeChange; + + // Retain ordering of any preceding id or class attributes + const idClassAttributeOrder = ownerElement[getOwnerElementIdClassNameAttrOrderingSym](); + for (const [attr] of idClassAttributeOrder) { + this[setNamedNodeMapValueSym](attr, ownerElement.getAttribute(attr) as string); + } } #attrNodeCache: Record = {}; @@ -409,7 +552,7 @@ export class NamedNodeMap { this.#map[safeAttrName] = value; if (bubble) { - this.#onAttrNodeChange(attribute, value); + this.#onAttrNodeChange(attribute, value, false); } } @@ -422,7 +565,7 @@ export class NamedNodeMap { if (this.#map[safeAttrName] !== undefined) { this.#length--; this.#map[safeAttrName] = undefined; - this.#onAttrNodeChange(attribute, null); + this.#onAttrNodeChange(attribute, null, true); const attrNode = this.#attrNodeCache[safeAttrName]; if (attrNode) { @@ -502,35 +645,81 @@ const xmlNamestartCharRe = new RegExp(`[${XML_NAMESTART_CHAR_RE_SRC}]`, "u"); const xmlNameCharRe = new RegExp(`[${XML_NAME_CHAR_RE_SRC}]`, "u"); export class Element extends Node { - localName: string; - attributes: NamedNodeMap = new NamedNodeMap(this, (attribute, value) => { - if (value === null) { - value = ""; - } + #namedNodeMap: NamedNodeMap | null = null; + get attributes(): NamedNodeMap { + if (!this.#namedNodeMap) { + this.#namedNodeMap = new NamedNodeMap(this, (attribute, value, isRemoved) => { + if (value === null) { + value = ""; + } - switch (attribute) { - case "class": - this.#classList.value = value; - break; - case "id": - this.#currentId = value; - break; + switch (attribute) { + case "class": + if (isRemoved) { + this.#hasClassNameAttribute = -1; + } else if (this.#hasClassNameAttribute === -1) { + this.#hasClassNameAttribute = this.#hasIdAttribute + 1; + } + // This must happen after the attribute is marked removed + this.#classList.value = value; + break; + case "id": + if (isRemoved) { + this.#hasIdAttribute = -1; + } else if (this.#hasIdAttribute === -1) { + this.#hasIdAttribute = this.#hasClassNameAttribute + 1; + } + this.#currentId = value; + break; + } + }, CTOR_KEY) } - }, CTOR_KEY); + + return this.#namedNodeMap; + } #datasetProxy: Record | null = null; #currentId = ""; - #classList = new DOMTokenList( - (className) => { - if (this.hasAttribute("class") || className !== "") { - this.attributes[setNamedNodeMapValueSym]("class", className); - } - }, - CTOR_KEY, - ); + #currentClassName = ""; + #hasIdAttribute = -1; + #hasClassNameAttribute = -1; + + /** + * Used when a NamedNodeMap is initialized so that it can persist the id and + * class attributes with the correct ordering + */ + [getOwnerElementIdClassNameAttrOrderingSym](): Array<[string, number]> { + const unordered: Array<[string, number]> = [ + ["id", this.#hasIdAttribute], + ["class", this.#hasClassNameAttribute], + ]; + return unordered + .filter(([_attr, order]) => Boolean(~order)) + .sort(([_a, orderA], [_b, orderB]) => orderA - orderB); + } + + // Only initialize a classList when we need one + #classListInstance: DOMTokenList | null = null; + #uninitializedClassListInstance: UinitializedDOMTokenList | null = new UinitializedDOMTokenList(this); + get #classList(): DOMTokenList { + return this.#classListInstance || this.#uninitializedClassListInstance as unknown as DOMTokenList; + } + + [initializeClassListSym]() { + this.#uninitializedClassListInstance = null; + this.#classListInstance = new DOMTokenList( + (className) => { + this.#currentClassName = className; + if (this.#namedNodeMap && (this.hasAttribute("class") || className !== "")) { + this.attributes[setNamedNodeMapValueSym]("class", className); + } + }, + CTOR_KEY, + ); + } constructor( - public tagName: string, + tagName: string, parentNode: Node | null, attributes: [string, string][], key: typeof CTOR_KEY, @@ -544,22 +733,20 @@ export class Element extends Node { for (const attr of attributes) { this.setAttribute(attr[0], attr[1]); - - switch (attr[0]) { - case "class": - this.#classList.value = attr[1]; - break; - case "id": - this.#currentId = attr[1]; - break; - } } - this.tagName = this.nodeName = tagName.toUpperCase(); - this.localName = tagName.toLowerCase(); + this.nodeName = getUpperCase(tagName); + } + + get tagName(): string { + return this.nodeName; + } + + get localName(): string { + return getLowerCase(this.tagName); } - _shallowClone(): Node { + override _shallowClone(): Node { // FIXME: This attribute copying needs to also be fixed in other // elements that override _shallowClone like `; diff --git a/src/dom/html-collection.ts b/src/dom/html-collection.ts index 6037eb0..174dd77 100644 --- a/src/dom/html-collection.ts +++ b/src/dom/html-collection.ts @@ -20,7 +20,7 @@ export const HTMLCollectionMutatorSym = Symbol("HTMLCollectionMutatorSym"); const HTMLCollectionClass: any = (() => { // @ts-ignore class HTMLCollection extends Array { - forEach( + override forEach( cb: (node: Element, index: number, nodeList: Element[]) => void, thisArg?: unknown, ) { @@ -41,7 +41,7 @@ const HTMLCollectionClass: any = (() => { }; } - toString() { + override toString() { return "[object HTMLCollection]"; } } diff --git a/src/dom/node-list.ts b/src/dom/node-list.ts index e6ddd32..c25b7cc 100644 --- a/src/dom/node-list.ts +++ b/src/dom/node-list.ts @@ -118,7 +118,7 @@ class NodeListMutatorImpl { const NodeListClass: any = (() => { // @ts-ignore class NodeList extends Array { - forEach( + override forEach( cb: (node: T, index: number, nodeList: T[]) => void, thisArg?: unknown, ) { @@ -141,7 +141,7 @@ const NodeListClass: any = (() => { } } - toString() { + override toString() { return "[object NodeList]"; } } diff --git a/src/dom/node.ts b/src/dom/node.ts index 64b2456..9baf9c8 100644 --- a/src/dom/node.ts +++ b/src/dom/node.ts @@ -57,12 +57,17 @@ export function nodesAndTextNodes( export class Node extends EventTarget { #nodeValue: string | null = null; - public childNodes: NodeList; public parentNode: Node | null = null; - public parentElement: Element | null; - #childNodesMutator: NodeListMutator; #ownerDocument: Document | null = null; - private _ancestors = new Set(); + private _ancestors: Set | null = null; + + get parentElement(): Element | null { + if (this.parentNode?.nodeType === NodeType.ELEMENT_NODE) { + return this.parentNode as Element; + } + + return null; + } // Instance constants defined after Node // class body below to avoid clutter @@ -91,17 +96,23 @@ export class Node extends EventTarget { super(); this.#nodeValue = null; - this.childNodes = new NodeList(); - this.#childNodesMutator = this.childNodes[nodeListMutatorSym](); - this.parentElement = parentNode; if (parentNode) { parentNode.appendChild(this); } } + #childNodes: NodeList | null = null; + get childNodes(): NodeList { + return this.#childNodes || (this.#childNodes = new NodeList()); + } + _getChildNodesMutator(): NodeListMutator { - return this.#childNodesMutator; + return this.childNodes[nodeListMutatorSym](); + } + + _hasInitializedChildNodes(): boolean { + return Boolean(this.#childNodes); } /** @@ -117,28 +128,25 @@ export class Node extends EventTarget { if (newParent) { if (!sameParent) { - // If this a document node or another non-element node - // then parentElement should be set to null - if (newParent.nodeType === NodeType.ELEMENT_NODE) { - this.parentElement = newParent as unknown as Element; - } else { - this.parentElement = null; - } - this._setOwnerDocument(newParent.#ownerDocument); } // Add parent chain to ancestors - this._ancestors = new Set(newParent._ancestors); - this._ancestors.add(newParent); + if (this.nodeType !== NodeType.TEXT_NODE) { + // this._ancestors = new Set(newParent._ancestors); + // this._ancestors.add(newParent); + } } else { - this.parentElement = null; - this._ancestors.clear(); + if (this._ancestors) { + this._ancestors.clear(); + } } // Update ancestors for child nodes - for (const child of this.childNodes) { - child._setParent(this, shouldUpdateParentAndAncestors); + if (this._hasInitializedChildNodes()) { + for (const child of this.childNodes) { + child._setParent(this, shouldUpdateParentAndAncestors); + } } } } @@ -154,14 +162,26 @@ export class Node extends EventTarget { if (this.#ownerDocument !== document) { this.#ownerDocument = document; - for (const child of this.childNodes) { - child._setOwnerDocument(document); + if (this._hasInitializedChildNodes()) { + for (const child of this.childNodes) { + child._setOwnerDocument(document); + } } } } contains(child: Node): boolean { - return child._ancestors.has(this) || child === this; + return false; + // if (!child._ancestors) { + // if (child.parentNode) { + // child._ancestors = new Set(child.parentNode._ancestors); + // child._ancestors.add(child.parentNode); + // } else { + // child._ancestors = new Set(); + // } + // } + + // return child._ancestors.has(this) || child === this; } get ownerDocument(): Document | null { @@ -203,15 +223,23 @@ export class Node extends EventTarget { } get firstChild(): Node | null { + if (!this._hasInitializedChildNodes()) { + return null; + } + return this.childNodes[0] || null; } get lastChild(): Node | null { + if (!this._hasInitializedChildNodes()) { + return null; + } + return this.childNodes[this.childNodes.length - 1] || null; } hasChildNodes(): boolean { - return Boolean(this.childNodes.length); + return this._hasInitializedChildNodes() && Boolean(this.childNodes.length); } cloneNode(deep = false): Node { @@ -219,7 +247,7 @@ export class Node extends EventTarget { copy._setOwnerDocument(this.ownerDocument); - if (deep) { + if (deep && this._hasInitializedChildNodes()) { for (const child of this.childNodes) { copy.appendChild(child.cloneNode(true)); } @@ -563,11 +591,11 @@ export class CharacterData extends Node { this.#nodeValue = data; } - get nodeValue(): string { + override get nodeValue(): string { return this.#nodeValue; } - set nodeValue(value: any) { + override set nodeValue(value: any) { this.#nodeValue = String(value ?? ""); } @@ -579,11 +607,11 @@ export class CharacterData extends Node { this.nodeValue = value; } - get textContent(): string { + override get textContent(): string { return this.#nodeValue; } - set textContent(value: any) { + override set textContent(value: any) { this.nodeValue = value; } @@ -628,7 +656,7 @@ export class Text extends CharacterData { ); } - _shallowClone(): Node { + override _shallowClone(): Node { return new Text(this.textContent); } } @@ -646,11 +674,11 @@ export class Comment extends CharacterData { ); } - _shallowClone(): Node { + override _shallowClone(): Node { return new Comment(this.textContent); } - get textContent(): string { + override get textContent(): string { return this.nodeValue; } } diff --git a/src/dom/string-cache.ts b/src/dom/string-cache.ts new file mode 100644 index 0000000..448837f --- /dev/null +++ b/src/dom/string-cache.ts @@ -0,0 +1,12 @@ +const upperCasedStringCache: Map = new Map(); +const lowerCasedStringCache: Map = new Map(); + +export function getUpperCase(string: string): string { + return upperCasedStringCache.get(string) + ?? upperCasedStringCache.set(string, string.toUpperCase()).get(string)!; +} + +export function getLowerCase(string: string): string { + return lowerCasedStringCache.get(string) + ?? lowerCasedStringCache.set(string, string.toLowerCase()).get(string)!; +} diff --git a/src/dom/utils.ts b/src/dom/utils.ts index 30290c4..b793970 100644 --- a/src/dom/utils.ts +++ b/src/dom/utils.ts @@ -1,6 +1,7 @@ import { Comment, Node, nodesAndTextNodes, NodeType, Text } from "./node.ts"; import { NodeList } from "./node-list.ts"; import UtilTypes from "./utils-types.ts"; +import { getLowerCase } from "./string-cache.ts"; import type { Element } from "./element.ts"; import type { HTMLTemplateElement } from "./elements/html-template-element.ts"; import type { DocumentFragment } from "./document-fragment.ts"; @@ -190,15 +191,13 @@ export function getOuterOrInnerHtml( return outerHTMLOpeningTag + innerHTML; } -// FIXME: This uses the incorrect .attributes implementation, it -// should probably be changed when .attributes is fixed export function getElementAttributesString( element: Element, ): string { let out = ""; for (const attribute of element.getAttributeNames()) { - out += ` ${attribute.toLowerCase()}`; + out += ` ${getLowerCase(attribute)}`; // escaping: https://html.spec.whatwg.org/multipage/parsing.html#escapingString out += `="${ From e34cd822bea68c65b6ca876ae671ede92d4b3c04 Mon Sep 17 00:00:00 2001 From: b-fuze Date: Mon, 25 Nov 2024 16:21:23 -0500 Subject: [PATCH 2/6] fix: classList changes not updating attribute --- src/dom/element.ts | 113 ++++++++++++++++++++++---------- test/units/Element-classList.ts | 16 ++++- 2 files changed, 91 insertions(+), 38 deletions(-) diff --git a/src/dom/element.ts b/src/dom/element.ts index d85fc69..c3841c5 100644 --- a/src/dom/element.ts +++ b/src/dom/element.ts @@ -207,8 +207,9 @@ export class DOMTokenList { oldToken: string, newToken: string, ): boolean { - const removeMethod = (this.#set.constructor === Array ? this.#arrayRemove : this.#setRemove).bind(this); - const addMethod = (this.#set.constructor === Array ? this.#arrayAdd : this.#setAdd).bind(this); + const isArrayBacked = this.#set.constructor === Array; + const removeMethod = (isArrayBacked ? this.#arrayRemove : this.#setRemove).bind(this); + const addMethod = (isArrayBacked ? this.#arrayAdd : this.#setAdd).bind(this); if ([oldToken, newToken].some((v) => DOMTokenList.#invalidToken(v))) { throw new DOMException( "Failed to execute 'replace' on 'DOMTokenList': The token provided must not be empty.", @@ -261,7 +262,6 @@ export class DOMTokenList { this.#value = Array.from(this.#set).join(" "); if (this.#set.constructor === Array && this.#set.length > DOMTokenList.#DOM_TOKEN_LIST_MIN_SET_SIZE) { - throw new Error("how: " + this.#set.length + " " + this.#set.join(",")); this.#set = new Set(this.#set); } } @@ -269,7 +269,13 @@ export class DOMTokenList { const initializeClassListSym = Symbol("initializeClassListSym"); const domTokenListCurrentElementSym = Symbol("domTokenListCurrentElementSym"); -class UinitializedDOMTokenList { +/** + * The purpose of this uninitialized DOMTokenList is to consume less memory + * than the actual DOMTokenList class. By measurements of Deno v2.1.0 (V8 13.0.245.12-rusty) + * this class consumes 48 bytes while the smallest DOMTokenList consumes 488 + * bytes + */ +class UninitializedDOMTokenList { // This will always be populated with the current element // being queried @@ -280,35 +286,57 @@ class UinitializedDOMTokenList { this[domTokenListCurrentElementSym] = currentElement; } - set value(input: string) { - if (input?.trim?.() !== "") { - this[domTokenListCurrentElementSym][initializeClassListSym](); - this[domTokenListCurrentElementSym].classList.value = input; + #getInitialized(): DOMTokenList | null { + const currentClassList = this[domTokenListCurrentElementSym].classList; + if (currentClassList === this as unknown as DOMTokenList) { + return null; } + + return currentClassList; + } + + set value(input: string) { + this[domTokenListCurrentElementSym][initializeClassListSym](); + this[domTokenListCurrentElementSym].classList.value = String(input); } get value(): string { - return ""; + return this.#getInitialized()?.value ?? ""; } get length(): number { - return 0; + return this.#getInitialized()?.length ?? 0; } - *entries(): IterableIterator<[number, string]> {} - *values(): IterableIterator {} - *keys(): IterableIterator {} + *entries(): IterableIterator<[number, string]> { + const initialized = this.#getInitialized(); + if (initialized) { + yield* initialized.entries(); + } + } + *values(): IterableIterator { + const initialized = this.#getInitialized(); + if (initialized) { + yield* initialized.values(); + } + } + *keys(): IterableIterator { + const initialized = this.#getInitialized(); + if (initialized) { + yield* initialized.keys(); + } + } *[Symbol.iterator](): IterableIterator { yield* this.values(); } - item(_index: number): string | null { - return null; + item(index: number): string | null { + return this.#getInitialized()?.item(index) ?? null; } - contains(_element: string): boolean { - return false; + contains(element: string): boolean { + return this.#getInitialized()?.contains(element) ?? false; } add(...elements: string[]) { @@ -316,9 +344,11 @@ class UinitializedDOMTokenList { this[domTokenListCurrentElementSym].classList.add(...elements); } - remove(..._elements: string[]) {} - replace(_oldToken: string, _newToken: string) { - return false; + remove(...elements: string[]) { + this.#getInitialized()?.remove(...elements); + } + replace(oldToken: string, newToken: string) { + return this.#getInitialized()?.replace(oldToken, newToken) ?? false; } supports(): never { @@ -330,7 +360,7 @@ class UinitializedDOMTokenList { force?: boolean, ): boolean { if (force === false) { - return false; + return this.#getInitialized()?.toggle(element, force) ?? false; } this[domTokenListCurrentElementSym][initializeClassListSym](); @@ -339,9 +369,11 @@ class UinitializedDOMTokenList { } forEach( - _callback: (value: string, index: number, list: DOMTokenList) => void, - ) {} -}; + callback: (value: string, index: number, list: DOMTokenList) => void, + ) { + this.#getInitialized()?.forEach(callback); + } +} const setNamedNodeMapOwnerElementSym = Symbol("setNamedNodeMapOwnerElementSym"); const setAttrValueSym = Symbol("setAttrValueSym"); @@ -474,11 +506,11 @@ export class NamedNodeMap { ?.slice(1); // Remove "a" for safeAttrName return this[getNamedNodeMapAttrNodeSym](attribute); }; - #onAttrNodeChange: (attr: string, value: string | null, isRemoved: boolean) => void; + #onAttrNodeChange: (attr: string, value: string | null) => void; constructor( ownerElement: Element, - onAttrNodeChange: (attr: string, value: string | null, isRemoved: boolean) => void, + onAttrNodeChange: (attr: string, value: string | null) => void, key: typeof CTOR_KEY, ) { if (key !== CTOR_KEY) { @@ -552,7 +584,7 @@ export class NamedNodeMap { this.#map[safeAttrName] = value; if (bubble) { - this.#onAttrNodeChange(attribute, value, false); + this.#onAttrNodeChange(attribute, value); } } @@ -565,7 +597,7 @@ export class NamedNodeMap { if (this.#map[safeAttrName] !== undefined) { this.#length--; this.#map[safeAttrName] = undefined; - this.#onAttrNodeChange(attribute, null, true); + this.#onAttrNodeChange(attribute, null); const attrNode = this.#attrNodeCache[safeAttrName]; if (attrNode) { @@ -648,7 +680,8 @@ export class Element extends Node { #namedNodeMap: NamedNodeMap | null = null; get attributes(): NamedNodeMap { if (!this.#namedNodeMap) { - this.#namedNodeMap = new NamedNodeMap(this, (attribute, value, isRemoved) => { + this.#namedNodeMap = new NamedNodeMap(this, (attribute, value) => { + const isRemoved = value === null; if (value === null) { value = ""; } @@ -661,6 +694,7 @@ export class Element extends Node { this.#hasClassNameAttribute = this.#hasIdAttribute + 1; } // This must happen after the attribute is marked removed + this.#currentClassName = value; this.#classList.value = value; break; case "id": @@ -699,19 +733,26 @@ export class Element extends Node { } // Only initialize a classList when we need one - #classListInstance: DOMTokenList | null = null; - #uninitializedClassListInstance: UinitializedDOMTokenList | null = new UinitializedDOMTokenList(this); + #classListInstance: DOMTokenList = new UninitializedDOMTokenList(this) as unknown as DOMTokenList; get #classList(): DOMTokenList { - return this.#classListInstance || this.#uninitializedClassListInstance as unknown as DOMTokenList; + return this.#classListInstance; } [initializeClassListSym]() { - this.#uninitializedClassListInstance = null; + if (this.#classListInstance.constructor === DOMTokenList) { + return; + } + this.#classListInstance = new DOMTokenList( (className) => { - this.#currentClassName = className; - if (this.#namedNodeMap && (this.hasAttribute("class") || className !== "")) { - this.attributes[setNamedNodeMapValueSym]("class", className); + if (this.#currentClassName !== className) { + this.#currentClassName = className; + if (this.#hasClassNameAttribute === -1) { + this.#hasClassNameAttribute = this.#hasIdAttribute + 1; + } + if (this.#namedNodeMap && (this.hasAttribute("class") || className !== "")) { + this.attributes[setNamedNodeMapValueSym]("class", className); + } } }, CTOR_KEY, diff --git a/test/units/Element-classList.ts b/test/units/Element-classList.ts index 88b1afe..9bd8afb 100644 --- a/test/units/Element-classList.ts +++ b/test/units/Element-classList.ts @@ -8,10 +8,12 @@ import { Deno.test("Element.classList.value", () => { const doc = new DOMParser().parseFromString( - "
", + "
", "text/html", ); const div = doc.querySelector("div")!; + const main = doc.querySelector("main")!; + main.classList.value = "a b"; assertStrictEquals( div.classList.value, "a b", @@ -53,6 +55,11 @@ Deno.test("Element.classList.value", () => { "c d e", `div.classList.value should be "c d e", got "${div.classList.value}"`, ); + assertStrictEquals( + main.getAttribute("class"), + "a b", + "main.classList.value = ... should set the 'class' attribute", + ); }); Deno.test("Element.classList.length", () => { @@ -89,7 +96,7 @@ Deno.test("Element.classList.length", () => { }); Deno.test("Element.classList.add", () => { - const doc = new DOMParser().parseFromString("
", "text/html"); + const doc = new DOMParser().parseFromString("
", "text/html"); const div = doc.querySelector("div")!; div.classList.add("a"); div.classList.add("b", "c"); @@ -117,6 +124,11 @@ Deno.test("Element.classList.add", () => { "The token provided must not be empty", "DOMTokenList.add() shouldn't accept an empty string.", ); + assertStrictEquals( + div.getAttribute("class"), + "a b c", + "div.classList.add(...) should set the 'class' attribute", + ); }); Deno.test("Element.classList.remove", () => { From 971b52356f16c86226228dd88c91afaa1a7019a3 Mon Sep 17 00:00:00 2001 From: b-fuze Date: Mon, 25 Nov 2024 17:14:37 -0500 Subject: [PATCH 3/6] fix: ancestor check --- src/dom/node.ts | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/src/dom/node.ts b/src/dom/node.ts index 9baf9c8..ae0b35a 100644 --- a/src/dom/node.ts +++ b/src/dom/node.ts @@ -38,7 +38,7 @@ export function nodesAndTextNodes( moveDocumentFragmentChildren(n as DocumentFragment, parentNode); return children; } else { - const node: Node = n instanceof Node ? n : new Text("" + n); + const node: Node = n instanceof Node ? n : new Text(String(n)); // Make sure the node isn't an ancestor of parentNode if (n === node && parentNode) { @@ -59,7 +59,6 @@ export class Node extends EventTarget { #nodeValue: string | null = null; public parentNode: Node | null = null; #ownerDocument: Document | null = null; - private _ancestors: Set | null = null; get parentElement(): Element | null { if (this.parentNode?.nodeType === NodeType.ELEMENT_NODE) { @@ -130,16 +129,6 @@ export class Node extends EventTarget { if (!sameParent) { this._setOwnerDocument(newParent.#ownerDocument); } - - // Add parent chain to ancestors - if (this.nodeType !== NodeType.TEXT_NODE) { - // this._ancestors = new Set(newParent._ancestors); - // this._ancestors.add(newParent); - } - } else { - if (this._ancestors) { - this._ancestors.clear(); - } } // Update ancestors for child nodes @@ -171,17 +160,16 @@ export class Node extends EventTarget { } contains(child: Node): boolean { + let node: Node | null = child; + while (node) { + if (node === this) { + return true; + } + + node = node.parentNode; + } + return false; - // if (!child._ancestors) { - // if (child.parentNode) { - // child._ancestors = new Set(child.parentNode._ancestors); - // child._ancestors.add(child.parentNode); - // } else { - // child._ancestors = new Set(); - // } - // } - - // return child._ancestors.has(this) || child === this; } get ownerDocument(): Document | null { From fa44f53a2d27275a7fdca8a33f8f3188e656b80a Mon Sep 17 00:00:00 2001 From: b-fuze Date: Sun, 5 Jan 2025 19:08:41 -0500 Subject: [PATCH 4/6] chore: correct id/class attr order, unit test optimized id/class attr This work was the result of memory optimizations to avoid allocating NamedNodeMap objects when an element only has either `id` or `class` attributes. If any other attributes are added then a NamedNodeMap gets allocated. --- src/dom/document-fragment.ts | 2 +- src/dom/document.ts | 4 +- src/dom/element.ts | 72 +++++++++++++++++++++++------------- src/dom/utils.ts | 14 +++---- test/units/NamedNodeMap.ts | 57 ++++++++++++++++++++++++++++ test/units/cloneNode.ts | 32 ++++++++++++---- 6 files changed, 137 insertions(+), 44 deletions(-) diff --git a/src/dom/document-fragment.ts b/src/dom/document-fragment.ts index 074cb16..3307892 100644 --- a/src/dom/document-fragment.ts +++ b/src/dom/document-fragment.ts @@ -137,7 +137,7 @@ function documentFragmentGetElementsByClassName( this: DocumentFragment, className: string, ) { - return getElementsByClassName(this, className, []); + return getElementsByClassName(this, className.trim().split(/\s+/), []); } function documentFragmentGetElementsByTagNameWildcard( diff --git a/src/dom/document.ts b/src/dom/document.ts index bac955c..631e537 100644 --- a/src/dom/document.ts +++ b/src/dom/document.ts @@ -120,9 +120,7 @@ export class Document extends Node { public body: Element = null; public implementation: DOMImplementation; - // #lockState = false; #documentURI = "about:blank"; // TODO - #title = ""; #nwapi: SelectorApi | null = null; constructor() { @@ -402,7 +400,7 @@ export class Document extends Node { } getElementsByClassName(className: string): Element[] { - return getElementsByClassName(this, className, []); + return getElementsByClassName(this, className.trim().split(/\s+/), []) as Element[]; } hasFocus(): boolean { diff --git a/src/dom/element.ts b/src/dom/element.ts index c3841c5..fa1761d 100644 --- a/src/dom/element.ts +++ b/src/dom/element.ts @@ -489,7 +489,6 @@ const getNamedNodeMapValueSym = Symbol("getNamedNodeMapValueSym"); const getNamedNodeMapAttrNamesSym = Symbol("getNamedNodeMapAttrNamesSym"); const getNamedNodeMapAttrNodeSym = Symbol("getNamedNodeMapAttrNodeSym"); const removeNamedNodeMapAttrSym = Symbol("removeNamedNodeMapAttrSym"); -const getOwnerElementIdClassNameAttrOrderingSym = Symbol("getOwnerElementIdClassNameAttrOrderingSym"); export class NamedNodeMap { static #indexedAttrAccess = function ( this: NamedNodeMap, @@ -520,8 +519,7 @@ export class NamedNodeMap { this.#onAttrNodeChange = onAttrNodeChange; // Retain ordering of any preceding id or class attributes - const idClassAttributeOrder = ownerElement[getOwnerElementIdClassNameAttrOrderingSym](); - for (const [attr] of idClassAttributeOrder) { + for (const attr of ownerElement.getAttributeNames()) { this[setNamedNodeMapValueSym](attr, ownerElement.getAttribute(attr) as string); } } @@ -687,7 +685,7 @@ export class Element extends Node { } switch (attribute) { - case "class": + case "class": { if (isRemoved) { this.#hasClassNameAttribute = -1; } else if (this.#hasClassNameAttribute === -1) { @@ -696,15 +694,17 @@ export class Element extends Node { // This must happen after the attribute is marked removed this.#currentClassName = value; this.#classList.value = value; - break; - case "id": + break; + } + case "id": { if (isRemoved) { this.#hasIdAttribute = -1; } else if (this.#hasIdAttribute === -1) { this.#hasIdAttribute = this.#hasClassNameAttribute + 1; } this.#currentId = value; - break; + break; + } } }, CTOR_KEY) } @@ -718,20 +718,6 @@ export class Element extends Node { #hasIdAttribute = -1; #hasClassNameAttribute = -1; - /** - * Used when a NamedNodeMap is initialized so that it can persist the id and - * class attributes with the correct ordering - */ - [getOwnerElementIdClassNameAttrOrderingSym](): Array<[string, number]> { - const unordered: Array<[string, number]> = [ - ["id", this.#hasIdAttribute], - ["class", this.#hasClassNameAttribute], - ]; - return unordered - .filter(([_attr, order]) => Boolean(~order)) - .sort(([_a, orderA], [_b, orderB]) => orderA - orderB); - } - // Only initialize a classList when we need one #classListInstance: DOMTokenList = new UninitializedDOMTokenList(this) as unknown as DOMTokenList; get #classList(): DOMTokenList { @@ -1002,8 +988,26 @@ export class Element extends Node { getAttributeNames(): string[] { if (!this.#namedNodeMap) { const attributes = []; - ~this.#hasIdAttribute && attributes.push("id"); - ~this.#hasClassNameAttribute && attributes.push("class"); + + // We preserve the order of the "id" and "class" attributes when + // returning the list of names with an uninitialized NamedNodeMap + const startWithClassAttr = Number(this.#hasIdAttribute > this.#hasClassNameAttribute); + for (let i = 0; i < 2; i++) { + const attributeIdx = (i + startWithClassAttr) % 2; + switch (attributeIdx) { + // "id" attribute + case 0: { + ~this.#hasIdAttribute && attributes.push("id"); + break; + } + // "class" attribute + case 1: { + ~this.#hasClassNameAttribute && attributes.push("class"); + break; + } + } + } + return attributes; } @@ -1045,12 +1049,16 @@ export class Element extends Node { switch (name) { case "id": { this.#currentId = strValue; - this.#hasIdAttribute = this.#hasClassNameAttribute + 1; + if (this.#hasIdAttribute === -1) { + this.#hasIdAttribute = this.#hasClassNameAttribute + 1; + } break; } case "class": { this.#classList.value = strValue; - this.#hasClassNameAttribute = this.#hasIdAttribute + 1; + if (this.#hasClassNameAttribute === -1) { + this.#hasClassNameAttribute = this.#hasIdAttribute + 1; + } break; } default: { @@ -1273,6 +1281,10 @@ export class Element extends Node { } getElementsByTagName(tagName: string): Element[] { + if (!this._hasInitializedChildNodes()) { + return []; + } + const fixCaseTagName = getUpperCase(tagName); if (fixCaseTagName === "*") { @@ -1316,10 +1328,18 @@ export class Element extends Node { } getElementsByClassName(className: string): Element[] { - return getElementsByClassName(this, className, []); + if (!this._hasInitializedChildNodes()) { + return []; + } + + return getElementsByClassName(this, className.trim().split(/\s+/), []) as Element[]; } getElementsByTagNameNS(_namespace: string, localName: string): Element[] { + if (!this._hasInitializedChildNodes()) { + return []; + } + // TODO: Use namespace return this.getElementsByTagName(localName); } diff --git a/src/dom/utils.ts b/src/dom/utils.ts index b793970..8820623 100644 --- a/src/dom/utils.ts +++ b/src/dom/utils.ts @@ -43,26 +43,25 @@ export function getDatasetJavascriptName(name: string): string { export function getElementsByClassName( element: any, - className: string, + classNames: string[], search: Node[], ): Node[] { for (const child of element.childNodes) { if (child.nodeType === NodeType.ELEMENT_NODE) { - const classList = className.trim().split(/\s+/); let matchesCount = 0; - for (const singleClassName of classList) { - if (( child).classList.contains(singleClassName)) { + for (const singleClassName of classNames) { + if ((child as Element).classList.contains(singleClassName)) { matchesCount++; } } // ensure that all class names are present - if (matchesCount === classList.length) { + if (matchesCount === classNames.length) { search.push(child); } - getElementsByClassName( child, className, search); + getElementsByClassName(child as Element, classNames, search); } } @@ -197,7 +196,8 @@ export function getElementAttributesString( let out = ""; for (const attribute of element.getAttributeNames()) { - out += ` ${getLowerCase(attribute)}`; + // attribute names should already all be lower-case + out += ` ${attribute}`; // escaping: https://html.spec.whatwg.org/multipage/parsing.html#escapingString out += `="${ diff --git a/test/units/NamedNodeMap.ts b/test/units/NamedNodeMap.ts index 01021c9..7e97688 100644 --- a/test/units/NamedNodeMap.ts +++ b/test/units/NamedNodeMap.ts @@ -243,3 +243,60 @@ Deno.test("NamedNodeMap stores unsafe Javascript property names", () => { assertEquals(div.getAttribute("constructor"), "fizz"); assertEquals(div.getAttribute("__proto__"), "qux"); }); + +Deno.test("Uninitialized NamedNodeMap preserves ID and className attribute ordering", () => { + const doc = new DOMParser().parseFromString( + ` +
+
+
+
+
+
+ `, + "text/html", + ); + + const div1 = doc.querySelector("#foo")!; + const div2 = doc.querySelector("#fizz")!; + const div3 = doc.querySelector("#only")!; + const div4 = doc.querySelector(".only")!; + const div5 = doc.querySelector("[data-others]")!; + const div6 = doc.querySelector("[data-more-others]")!; + + assertDeepEquals(div1.getAttributeNames(), ["id", "class"]); + assertDeepEquals(div2.getAttributeNames(), ["class", "id"]); + assertDeepEquals(div3.getAttributeNames(), ["id"]); + assertDeepEquals(div4.getAttributeNames(), ["class"]); + assertDeepEquals(div5.getAttributeNames(), ["data-others", "class", "id"]); + assertDeepEquals(div6.getAttributeNames(), ["id", "data-more-others", "class"]); + + // Test that ordering is undisturbed when setting id/class a second time + div1.setAttribute("id", "div1-id"); + div2.setAttribute("class", "div2-class"); + + assertDeepEquals(div1.getAttributeNames(), ["id", "class"]); + assertDeepEquals(div2.getAttributeNames(), ["class", "id"]); + + // Test that ordering is altered when removing and re-adding + div1.removeAttribute("id"); + div2.removeAttribute("class"); + + assertDeepEquals(div1.getAttributeNames(), ["class"]); + assertDeepEquals(div2.getAttributeNames(), ["id"]); + + div1.setAttribute("id", "foo"); + div2.setAttribute("class", "baz"); + + assertDeepEquals(div1.getAttributeNames(), ["class", "id"]); + assertDeepEquals(div2.getAttributeNames(), ["id", "class"]); + + // Test attribute ordering is preserved after NamedNodeMap initialization + div1.setAttribute("data-fizz", ""); + div2.removeAttribute("id"); + div2.setAttribute("data-foo", ""); + div2.setAttribute("id", "new-id"); + + assertDeepEquals(div1.getAttributeNames(), ["class", "id", "data-fizz"]); + assertDeepEquals(div2.getAttributeNames(), ["class", "data-foo", "id"]); +}); diff --git a/test/units/cloneNode.ts b/test/units/cloneNode.ts index c20c42f..3561c84 100644 --- a/test/units/cloneNode.ts +++ b/test/units/cloneNode.ts @@ -2,24 +2,42 @@ import { DOMParser, Element, Node } from "../../deno-dom-wasm.ts"; import { assert, assertEquals, - assertNotEquals, } from "https://deno.land/std@0.85.0/testing/asserts.ts"; Deno.test("cloneNode", () => { const doc = new DOMParser().parseFromString( ` - a -

b

-
  • c
- - e - `, + a +

b

+
  • c
+ + e + `, "text/html", ); checkClone(doc, doc.cloneNode(true)); }); +Deno.test("cloneNode works with uninitialized NamedNodeMap", () => { + const doc = new DOMParser().parseFromString( + ` +
+
+
+ `, + "text/html", + ); + + const div1 = doc.querySelector("#bar")!; + const div2 = doc.querySelector("#baz")!; + const div3 = doc.querySelector("#fizz")!; + + assertEquals(div1.getAttributeNames(), (div1.cloneNode() as Element).getAttributeNames()); + assertEquals(div2.getAttributeNames(), (div2.cloneNode() as Element).getAttributeNames()); + assertEquals(div3.getAttributeNames(), (div3.cloneNode() as Element).getAttributeNames()); +}); + function checkClone(node: Node, clone: Node) { assert(clone instanceof Node); assert(node !== clone); From 13e9b4e9d392b773d14987ca3b9af547ba77f934 Mon Sep 17 00:00:00 2001 From: b-fuze Date: Sun, 5 Jan 2025 19:10:24 -0500 Subject: [PATCH 5/6] chore: fmt --- .github/workflows/main.yml | 20 +++++++------- deno.jsonc | 1 + src/dom/document.ts | 6 +++- src/dom/element.ts | 49 +++++++++++++++++++++++++-------- src/dom/string-cache.ts | 8 +++--- test/units/Element-classList.ts | 5 +++- test/units/NamedNodeMap.ts | 6 +++- test/units/basic.html | 1 - test/units/cloneNode.ts | 15 ++++++++-- 9 files changed, 78 insertions(+), 33 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 6c78473..b5978a5 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -3,17 +3,17 @@ name: ci on: push: paths: - - 'Cargo.toml' - - 'html-parser/core/**' - - 'html-parser/plugin/**' - - '.github/workflows/**' + - "Cargo.toml" + - "html-parser/core/**" + - "html-parser/plugin/**" + - ".github/workflows/**" pull_request: branches-ignore: [master] paths: - - 'Cargo.toml' - - 'html-parser/core/**' - - 'html-parser/plugin/**' - - '.github/workflows/**' + - "Cargo.toml" + - "html-parser/core/**" + - "html-parser/plugin/**" + - ".github/workflows/**" jobs: build: @@ -36,7 +36,7 @@ jobs: - name: Install rust uses: hecrj/setup-rust-action@v1 with: - rust-version: '1.60.0' + rust-version: "1.60.0" - name: Log versions run: | @@ -143,7 +143,7 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: - tag_name: 'release draft' + tag_name: "release draft" draft: true files: | target/release/libplugin.dylib diff --git a/deno.jsonc b/deno.jsonc index 13eaefa..5951b8e 100644 --- a/deno.jsonc +++ b/deno.jsonc @@ -35,6 +35,7 @@ "exclude": [ "./wpt", "./bench/node_modules", + "./bench/c.html", "./src/dom/selectors/nwsapi.js", "./src/dom/selectors/sizzle.js", "./target", diff --git a/src/dom/document.ts b/src/dom/document.ts index 631e537..f009653 100644 --- a/src/dom/document.ts +++ b/src/dom/document.ts @@ -400,7 +400,11 @@ export class Document extends Node { } getElementsByClassName(className: string): Element[] { - return getElementsByClassName(this, className.trim().split(/\s+/), []) as Element[]; + return getElementsByClassName( + this, + className.trim().split(/\s+/), + [], + ) as Element[]; } hasFocus(): boolean { diff --git a/src/dom/element.ts b/src/dom/element.ts index fa1761d..7f2ed6d 100644 --- a/src/dom/element.ts +++ b/src/dom/element.ts @@ -13,7 +13,7 @@ import { upperCaseCharRe, } from "./utils.ts"; import UtilTypes from "./utils-types.ts"; -import { getUpperCase, getLowerCase } from "./string-cache.ts"; +import { getLowerCase, getUpperCase } from "./string-cache.ts"; export interface DOMTokenList { [index: number]: string; @@ -155,7 +155,10 @@ export class DOMTokenList { add( ...elements: Array ) { - const method = (this.#set.constructor === Array ? this.#arrayAdd : this.#setAdd).bind(this); + const method = + (this.#set.constructor === Array ? this.#arrayAdd : this.#setAdd).bind( + this, + ); for (const element of elements) { if (DOMTokenList.#invalidToken(element)) { @@ -183,7 +186,9 @@ export class DOMTokenList { remove( ...elements: Array ) { - const method = (this.#set.constructor === Array ? this.#arrayRemove : this.#setRemove).bind(this); + const method = + (this.#set.constructor === Array ? this.#arrayRemove : this.#setRemove) + .bind(this); const size = this.length; for (const element of elements) { if (DOMTokenList.#invalidToken(element)) { @@ -208,8 +213,11 @@ export class DOMTokenList { newToken: string, ): boolean { const isArrayBacked = this.#set.constructor === Array; - const removeMethod = (isArrayBacked ? this.#arrayRemove : this.#setRemove).bind(this); - const addMethod = (isArrayBacked ? this.#arrayAdd : this.#setAdd).bind(this); + const removeMethod = (isArrayBacked ? this.#arrayRemove : this.#setRemove) + .bind(this); + const addMethod = (isArrayBacked ? this.#arrayAdd : this.#setAdd).bind( + this, + ); if ([oldToken, newToken].some((v) => DOMTokenList.#invalidToken(v))) { throw new DOMException( "Failed to execute 'replace' on 'DOMTokenList': The token provided must not be empty.", @@ -261,7 +269,10 @@ export class DOMTokenList { #updateClassString() { this.#value = Array.from(this.#set).join(" "); - if (this.#set.constructor === Array && this.#set.length > DOMTokenList.#DOM_TOKEN_LIST_MIN_SET_SIZE) { + if ( + this.#set.constructor === Array && + this.#set.length > DOMTokenList.#DOM_TOKEN_LIST_MIN_SET_SIZE + ) { this.#set = new Set(this.#set); } } @@ -520,7 +531,10 @@ export class NamedNodeMap { // Retain ordering of any preceding id or class attributes for (const attr of ownerElement.getAttributeNames()) { - this[setNamedNodeMapValueSym](attr, ownerElement.getAttribute(attr) as string); + this[setNamedNodeMapValueSym]( + attr, + ownerElement.getAttribute(attr) as string, + ); } } @@ -706,7 +720,7 @@ export class Element extends Node { break; } } - }, CTOR_KEY) + }, CTOR_KEY); } return this.#namedNodeMap; @@ -719,7 +733,9 @@ export class Element extends Node { #hasClassNameAttribute = -1; // Only initialize a classList when we need one - #classListInstance: DOMTokenList = new UninitializedDOMTokenList(this) as unknown as DOMTokenList; + #classListInstance: DOMTokenList = new UninitializedDOMTokenList( + this, + ) as unknown as DOMTokenList; get #classList(): DOMTokenList { return this.#classListInstance; } @@ -736,7 +752,10 @@ export class Element extends Node { if (this.#hasClassNameAttribute === -1) { this.#hasClassNameAttribute = this.#hasIdAttribute + 1; } - if (this.#namedNodeMap && (this.hasAttribute("class") || className !== "")) { + if ( + this.#namedNodeMap && + (this.hasAttribute("class") || className !== "") + ) { this.attributes[setNamedNodeMapValueSym]("class", className); } } @@ -991,7 +1010,9 @@ export class Element extends Node { // We preserve the order of the "id" and "class" attributes when // returning the list of names with an uninitialized NamedNodeMap - const startWithClassAttr = Number(this.#hasIdAttribute > this.#hasClassNameAttribute); + const startWithClassAttr = Number( + this.#hasIdAttribute > this.#hasClassNameAttribute, + ); for (let i = 0; i < 2; i++) { const attributeIdx = (i + startWithClassAttr) % 2; switch (attributeIdx) { @@ -1332,7 +1353,11 @@ export class Element extends Node { return []; } - return getElementsByClassName(this, className.trim().split(/\s+/), []) as Element[]; + return getElementsByClassName( + this, + className.trim().split(/\s+/), + [], + ) as Element[]; } getElementsByTagNameNS(_namespace: string, localName: string): Element[] { diff --git a/src/dom/string-cache.ts b/src/dom/string-cache.ts index 448837f..fc07882 100644 --- a/src/dom/string-cache.ts +++ b/src/dom/string-cache.ts @@ -2,11 +2,11 @@ const upperCasedStringCache: Map = new Map(); const lowerCasedStringCache: Map = new Map(); export function getUpperCase(string: string): string { - return upperCasedStringCache.get(string) - ?? upperCasedStringCache.set(string, string.toUpperCase()).get(string)!; + return upperCasedStringCache.get(string) ?? + upperCasedStringCache.set(string, string.toUpperCase()).get(string)!; } export function getLowerCase(string: string): string { - return lowerCasedStringCache.get(string) - ?? lowerCasedStringCache.set(string, string.toLowerCase()).get(string)!; + return lowerCasedStringCache.get(string) ?? + lowerCasedStringCache.set(string, string.toLowerCase()).get(string)!; } diff --git a/test/units/Element-classList.ts b/test/units/Element-classList.ts index 9bd8afb..4c0b2ec 100644 --- a/test/units/Element-classList.ts +++ b/test/units/Element-classList.ts @@ -96,7 +96,10 @@ Deno.test("Element.classList.length", () => { }); Deno.test("Element.classList.add", () => { - const doc = new DOMParser().parseFromString("
", "text/html"); + const doc = new DOMParser().parseFromString( + "
", + "text/html", + ); const div = doc.querySelector("div")!; div.classList.add("a"); div.classList.add("b", "c"); diff --git a/test/units/NamedNodeMap.ts b/test/units/NamedNodeMap.ts index 7e97688..e53af35 100644 --- a/test/units/NamedNodeMap.ts +++ b/test/units/NamedNodeMap.ts @@ -269,7 +269,11 @@ Deno.test("Uninitialized NamedNodeMap preserves ID and className attribute order assertDeepEquals(div3.getAttributeNames(), ["id"]); assertDeepEquals(div4.getAttributeNames(), ["class"]); assertDeepEquals(div5.getAttributeNames(), ["data-others", "class", "id"]); - assertDeepEquals(div6.getAttributeNames(), ["id", "data-more-others", "class"]); + assertDeepEquals(div6.getAttributeNames(), [ + "id", + "data-more-others", + "class", + ]); // Test that ordering is undisturbed when setting id/class a second time div1.setAttribute("id", "div1-id"); diff --git a/test/units/basic.html b/test/units/basic.html index a584ee1..f4eea0e 100644 --- a/test/units/basic.html +++ b/test/units/basic.html @@ -11,4 +11,3 @@

His fleece as white as snow

He was sure to go - diff --git a/test/units/cloneNode.ts b/test/units/cloneNode.ts index 3561c84..155b26e 100644 --- a/test/units/cloneNode.ts +++ b/test/units/cloneNode.ts @@ -33,9 +33,18 @@ Deno.test("cloneNode works with uninitialized NamedNodeMap", () => { const div2 = doc.querySelector("#baz")!; const div3 = doc.querySelector("#fizz")!; - assertEquals(div1.getAttributeNames(), (div1.cloneNode() as Element).getAttributeNames()); - assertEquals(div2.getAttributeNames(), (div2.cloneNode() as Element).getAttributeNames()); - assertEquals(div3.getAttributeNames(), (div3.cloneNode() as Element).getAttributeNames()); + assertEquals( + div1.getAttributeNames(), + (div1.cloneNode() as Element).getAttributeNames(), + ); + assertEquals( + div2.getAttributeNames(), + (div2.cloneNode() as Element).getAttributeNames(), + ); + assertEquals( + div3.getAttributeNames(), + (div3.cloneNode() as Element).getAttributeNames(), + ); }); function checkClone(node: Node, clone: Node) { From bea231a90b634358971a0191b6967833b2ee3870 Mon Sep 17 00:00:00 2001 From: b-fuze Date: Sun, 5 Jan 2025 19:57:51 -0500 Subject: [PATCH 6/6] doc: differences from standard APIs wrt `UninitializedDOMTokenList` --- README.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/README.md b/README.md index ee20a6f..ad9a2db 100644 --- a/README.md +++ b/README.md @@ -139,6 +139,37 @@ To use the new binary you need to set the **`DENO_DOM_PLUGIN`** env var to the path of the binary produced in the previous step. **Don't forget** to run Deno with `--allow-env`. +## Inconsistencies with standard APIs + +### Differences in `DOMTokenList`/`Element.classList` + +To optimize memory usage, Deno DOM doesn't allocate `DOMTokenList`s +(`Element.classList`) on `Element` creation, it instead creates an +`UninitializedDOMTokenList` object. This can cause a subtle deviation from +standard APIs: + +```typescript +const div = doc.createElement("div"); + +// Retrieve the uninitialized DOMTokenList +const { classList } = div; + +// Initialize the DOMTokenList by adding a few classes +classList.add("foo"); +classList.add("bar"); + +// Inconsistency: the uninitialized classList/DOMTokenList +// is now a different object from the initialized one + +classList !== div.classList; + +// However, the uninitialized DOMTokenList object still +// works as the initialized DOMTokenList object: + +classList.add("fizz"); +div.classList.contains("fizz") === true; +``` + # Credits - html5ever developers for the HTML parser