From 3ef12df6641dbbb1989ce3d216c0b4e7d7f6a6ae Mon Sep 17 00:00:00 2001 From: tom pridham Date: Tue, 4 Jun 2024 18:55:59 -0600 Subject: [PATCH 1/9] feat: store getComputedStyle result to avoid redoing that work --- sources/__tests__/accessible-name.js | 9 ++++++++ sources/accessible-name-and-description.ts | 24 ++++++++++++++++------ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/sources/__tests__/accessible-name.js b/sources/__tests__/accessible-name.js index 3d01488c..39d042fe 100644 --- a/sources/__tests__/accessible-name.js +++ b/sources/__tests__/accessible-name.js @@ -591,6 +591,15 @@ describe("options.getComputedStyle", () => { expect(name).toEqual("foo test foo"); expect(window.getComputedStyle).not.toHaveBeenCalled(); }); + it("is not called more than necessary", () => { + const container = renderIntoDocument( + "" + ); + + computeAccessibleName(container.querySelector("button")); + + expect(window.getComputedStyle).toHaveBeenCalledTimes(4); + }); }); describe("options.computedStyleSupportsPseudoElements", () => { diff --git a/sources/accessible-name-and-description.ts b/sources/accessible-name-and-description.ts index 08d04476..918d7e17 100644 --- a/sources/accessible-name-and-description.ts +++ b/sources/accessible-name-and-description.ts @@ -65,11 +65,13 @@ function asFlatString(s: string): FlatString { * * @param node - * @param options - These are not optional to prevent accidentally calling it without options in `computeAccessibleName` + * @param maybeNodeStyle - if we call `getComputedStyle` we can store the result here * @returns {boolean} - */ function isHidden( node: Node, getComputedStyleImplementation: typeof window.getComputedStyle, + maybeNodeStyle: { current: CSSStyleDeclaration | null } ): node is Element { if (!isElement(node)) { return false; @@ -83,6 +85,7 @@ function isHidden( } const style = getComputedStyleImplementation(node); + maybeNodeStyle.current = style; return ( style.getPropertyValue("display") === "none" || style.getPropertyValue("visibility") === "hidden" @@ -355,7 +358,12 @@ export function computeTextAlternative( // 2F.i function computeMiscTextAlternative( node: Node, - context: { isEmbeddedInLabel: boolean; isReferenced: boolean }, + context: { + isEmbeddedInLabel: boolean; + isReferenced: boolean; + // if we called getComputedStyle earlier in the stack, we can pass it here to avoid calling it again + style: CSSStyleDeclaration | null; + } ): string { let accumulatedText = ""; if (isElement(node) && computedStyleSupportsPseudoElements) { @@ -378,13 +386,12 @@ export function computeTextAlternative( // TODO: Unclear why display affects delimiter // see https://github.com/w3c/accname/issues/3 const display = isElement(child) - ? getComputedStyle(child).getPropertyValue("display") + ? (context.style || getComputedStyle(child)).getPropertyValue("display") : "inline"; const separator = display !== "inline" ? " " : ""; // trailing separator for wpt tests accumulatedText += `${separator}${result}${separator}`; }); - if (isElement(node) && computedStyleSupportsPseudoElements) { const pseudoAfter = getComputedStyle(node, "::after"); const afterContent = getTextualContent(pseudoAfter); @@ -544,6 +551,7 @@ export function computeTextAlternative( const nameFromSubTree = computeMiscTextAlternative(node, { isEmbeddedInLabel: false, isReferenced: false, + style: null, }); if (nameFromSubTree !== "") { return nameFromSubTree; @@ -559,16 +567,18 @@ export function computeTextAlternative( isEmbeddedInLabel: boolean; isReferenced: boolean; recursion: boolean; - }, + } ): string { if (consultedNodes.has(current)) { return ""; } - + let maybeNodeStyle: { current: CSSStyleDeclaration | null } = { + current: null, + }; // 2A if ( !hidden && - isHidden(current, getComputedStyle) && + isHidden(current, getComputedStyle, maybeNodeStyle) && !context.isReferenced ) { consultedNodes.add(current); @@ -687,6 +697,7 @@ export function computeTextAlternative( const accumulatedText2F = computeMiscTextAlternative(current, { isEmbeddedInLabel: context.isEmbeddedInLabel, isReferenced: false, + style: maybeNodeStyle.current, }); if (accumulatedText2F !== "") { consultedNodes.add(current); @@ -704,6 +715,7 @@ export function computeTextAlternative( return computeMiscTextAlternative(current, { isEmbeddedInLabel: context.isEmbeddedInLabel, isReferenced: false, + style: maybeNodeStyle.current, }); } From 93075c8312d4a6743d546d001ee5d2fc5a4b4f72 Mon Sep 17 00:00:00 2001 From: tom pridham Date: Fri, 7 Jun 2024 16:33:00 -0600 Subject: [PATCH 2/9] fix: use a caching version of getComputedStyles instead of passing mutable objects around --- sources/__tests__/accessible-name.js | 6 +- sources/accessible-name-and-description.ts | 45 ++++++++------- sources/polyfills/MapLike.ts | 64 ++++++++++++++++++++++ 3 files changed, 93 insertions(+), 22 deletions(-) create mode 100644 sources/polyfills/MapLike.ts diff --git a/sources/__tests__/accessible-name.js b/sources/__tests__/accessible-name.js index 39d042fe..25380988 100644 --- a/sources/__tests__/accessible-name.js +++ b/sources/__tests__/accessible-name.js @@ -591,14 +591,14 @@ describe("options.getComputedStyle", () => { expect(name).toEqual("foo test foo"); expect(window.getComputedStyle).not.toHaveBeenCalled(); }); - it("is not called more than necessary", () => { + it("is not called more than once per element", () => { const container = renderIntoDocument( "" ); computeAccessibleName(container.querySelector("button")); - - expect(window.getComputedStyle).toHaveBeenCalledTimes(4); + // once for the button, once for each span + expect(window.getComputedStyle).toHaveBeenCalledTimes(3); }); }); diff --git a/sources/accessible-name-and-description.ts b/sources/accessible-name-and-description.ts index 918d7e17..3550e3a9 100644 --- a/sources/accessible-name-and-description.ts +++ b/sources/accessible-name-and-description.ts @@ -3,6 +3,7 @@ */ import ArrayFrom from "./polyfills/array.from"; import SetLike from "./polyfills/SetLike"; +import MapLike from "./polyfills/MapLike"; import { hasAnyConcreteRoles, isElement, @@ -29,6 +30,7 @@ import { type FlatString = string & { __flat: true; }; +type GetComputedStyle = typeof window.getComputedStyle; /** * interface for an options-bag where `window.getComputedStyle` can be mocked @@ -43,7 +45,7 @@ export interface ComputeTextAlternativeOptions { /** * mock window.getComputedStyle. Needs `content`, `display` and `visibility` */ - getComputedStyle?: typeof window.getComputedStyle; + getComputedStyle?: GetComputedStyle; /** * Set to `true` if you want to include hidden elements in the accessible name and description computation. * Skips 2A in https://w3c.github.io/accname/#computation-steps. @@ -65,13 +67,11 @@ function asFlatString(s: string): FlatString { * * @param node - * @param options - These are not optional to prevent accidentally calling it without options in `computeAccessibleName` - * @param maybeNodeStyle - if we call `getComputedStyle` we can store the result here * @returns {boolean} - */ function isHidden( node: Node, - getComputedStyleImplementation: typeof window.getComputedStyle, - maybeNodeStyle: { current: CSSStyleDeclaration | null } + getComputedStyleImplementation: GetComputedStyle ): node is Element { if (!isElement(node)) { return false; @@ -85,7 +85,6 @@ function isHidden( } const style = getComputedStyleImplementation(node); - maybeNodeStyle.current = style; return ( style.getPropertyValue("display") === "none" || style.getPropertyValue("visibility") === "hidden" @@ -338,9 +337,10 @@ function getSlotContents(slot: HTMLSlotElement): Node[] { */ export function computeTextAlternative( root: Element, - options: ComputeTextAlternativeOptions = {}, + options: ComputeTextAlternativeOptions = {} ): string { const consultedNodes = new SetLike(); + const computedStyles = new MapLike(); const window = safeWindow(root); const { @@ -351,9 +351,24 @@ export function computeTextAlternative( // window.getComputedStyle(elementFromAnotherWindow) or if I don't bind it // the type declarations don't require a `this` // eslint-disable-next-line no-restricted-properties - getComputedStyle = window.getComputedStyle.bind(window), + getComputedStyle: _getComputedStyle = window.getComputedStyle.bind(window), hidden = false, } = options; + const cachingGetComputedStyle: GetComputedStyle = ( + el, + pseudoElement + ): CSSStyleDeclaration => { + // we don't cache the pseudoElement styles + if (pseudoElement !== undefined) { + return _getComputedStyle(el, pseudoElement); + } + if (computedStyles.has(el)) { + return computedStyles.get(el)!; + } + const style = _getComputedStyle(el, pseudoElement); + computedStyles.set(el, style); + return style; + }; // 2F.i function computeMiscTextAlternative( @@ -361,13 +376,11 @@ export function computeTextAlternative( context: { isEmbeddedInLabel: boolean; isReferenced: boolean; - // if we called getComputedStyle earlier in the stack, we can pass it here to avoid calling it again - style: CSSStyleDeclaration | null; } ): string { let accumulatedText = ""; if (isElement(node) && computedStyleSupportsPseudoElements) { - const pseudoBefore = getComputedStyle(node, "::before"); + const pseudoBefore = cachingGetComputedStyle(node, "::before"); const beforeContent = getTextualContent(pseudoBefore); accumulatedText = `${beforeContent} ${accumulatedText}`; } @@ -386,14 +399,14 @@ export function computeTextAlternative( // TODO: Unclear why display affects delimiter // see https://github.com/w3c/accname/issues/3 const display = isElement(child) - ? (context.style || getComputedStyle(child)).getPropertyValue("display") + ? cachingGetComputedStyle(child).getPropertyValue("display") : "inline"; const separator = display !== "inline" ? " " : ""; // trailing separator for wpt tests accumulatedText += `${separator}${result}${separator}`; }); if (isElement(node) && computedStyleSupportsPseudoElements) { - const pseudoAfter = getComputedStyle(node, "::after"); + const pseudoAfter = cachingGetComputedStyle(node, "::after"); const afterContent = getTextualContent(pseudoAfter); accumulatedText = `${accumulatedText} ${afterContent}`; } @@ -551,7 +564,6 @@ export function computeTextAlternative( const nameFromSubTree = computeMiscTextAlternative(node, { isEmbeddedInLabel: false, isReferenced: false, - style: null, }); if (nameFromSubTree !== "") { return nameFromSubTree; @@ -572,13 +584,10 @@ export function computeTextAlternative( if (consultedNodes.has(current)) { return ""; } - let maybeNodeStyle: { current: CSSStyleDeclaration | null } = { - current: null, - }; // 2A if ( !hidden && - isHidden(current, getComputedStyle, maybeNodeStyle) && + isHidden(current, cachingGetComputedStyle) && !context.isReferenced ) { consultedNodes.add(current); @@ -697,7 +706,6 @@ export function computeTextAlternative( const accumulatedText2F = computeMiscTextAlternative(current, { isEmbeddedInLabel: context.isEmbeddedInLabel, isReferenced: false, - style: maybeNodeStyle.current, }); if (accumulatedText2F !== "") { consultedNodes.add(current); @@ -715,7 +723,6 @@ export function computeTextAlternative( return computeMiscTextAlternative(current, { isEmbeddedInLabel: context.isEmbeddedInLabel, isReferenced: false, - style: maybeNodeStyle.current, }); } diff --git a/sources/polyfills/MapLike.ts b/sources/polyfills/MapLike.ts new file mode 100644 index 00000000..58dc2ef8 --- /dev/null +++ b/sources/polyfills/MapLike.ts @@ -0,0 +1,64 @@ +declare global { + class Map { + // es2015.collection.d.ts + clear(): void; + delete(key: K): boolean; + forEach( + callbackfn: (value: V, key: K, map: Map) => void, + thisArg?: any + ): void; + get(key: K): V | undefined; + has(key: K): boolean; + set(key: K, value: V): this; + readonly size: number; + } +} + +class MapLike { + private keys: K[]; + private values: V[]; + + constructor() { + this.keys = []; + this.values = []; + } + clear() { + this.keys = []; + this.values = []; + } + delete(key: K): boolean { + const index = this.keys.indexOf(key); + if (index === -1) { + return false; + } + this.keys.splice(index, 1); + this.values.splice(index, 1); + return true; + } + get(key: K): V | undefined { + const index = this.keys.indexOf(key); + if (index === -1) { + return undefined; + } + return this.values[index]; + } + has(key: K): boolean { + return this.keys.indexOf(key) !== -1; + } + set(key: K, value: V): this { + const index = this.keys.indexOf(key); + if (index === -1) { + this.keys.push(key); + this.values.push(value); + } else { + this.values[index] = value; + } + return this; + } + forEach( + callbackfn: (value: V, key: K, map: MapLike) => void, + thisArg?: any + ): void {} +} + +export default typeof Map === "undefined" ? Map: MapLike; From 8ea82e3dd249564081e4fba2396f36f82a16f539 Mon Sep 17 00:00:00 2001 From: tom pridham Date: Fri, 7 Jun 2024 16:34:00 -0600 Subject: [PATCH 3/9] fix: run format --- sources/__tests__/accessible-name.js | 2 +- sources/accessible-name-and-description.ts | 10 +++++----- sources/polyfills/MapLike.ts | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/sources/__tests__/accessible-name.js b/sources/__tests__/accessible-name.js index 25380988..b1477312 100644 --- a/sources/__tests__/accessible-name.js +++ b/sources/__tests__/accessible-name.js @@ -593,7 +593,7 @@ describe("options.getComputedStyle", () => { }); it("is not called more than once per element", () => { const container = renderIntoDocument( - "" + "", ); computeAccessibleName(container.querySelector("button")); diff --git a/sources/accessible-name-and-description.ts b/sources/accessible-name-and-description.ts index 3550e3a9..2d8a9148 100644 --- a/sources/accessible-name-and-description.ts +++ b/sources/accessible-name-and-description.ts @@ -71,7 +71,7 @@ function asFlatString(s: string): FlatString { */ function isHidden( node: Node, - getComputedStyleImplementation: GetComputedStyle + getComputedStyleImplementation: GetComputedStyle, ): node is Element { if (!isElement(node)) { return false; @@ -337,7 +337,7 @@ function getSlotContents(slot: HTMLSlotElement): Node[] { */ export function computeTextAlternative( root: Element, - options: ComputeTextAlternativeOptions = {} + options: ComputeTextAlternativeOptions = {}, ): string { const consultedNodes = new SetLike(); const computedStyles = new MapLike(); @@ -356,7 +356,7 @@ export function computeTextAlternative( } = options; const cachingGetComputedStyle: GetComputedStyle = ( el, - pseudoElement + pseudoElement, ): CSSStyleDeclaration => { // we don't cache the pseudoElement styles if (pseudoElement !== undefined) { @@ -376,7 +376,7 @@ export function computeTextAlternative( context: { isEmbeddedInLabel: boolean; isReferenced: boolean; - } + }, ): string { let accumulatedText = ""; if (isElement(node) && computedStyleSupportsPseudoElements) { @@ -579,7 +579,7 @@ export function computeTextAlternative( isEmbeddedInLabel: boolean; isReferenced: boolean; recursion: boolean; - } + }, ): string { if (consultedNodes.has(current)) { return ""; diff --git a/sources/polyfills/MapLike.ts b/sources/polyfills/MapLike.ts index 58dc2ef8..099ceb90 100644 --- a/sources/polyfills/MapLike.ts +++ b/sources/polyfills/MapLike.ts @@ -5,7 +5,7 @@ declare global { delete(key: K): boolean; forEach( callbackfn: (value: V, key: K, map: Map) => void, - thisArg?: any + thisArg?: any, ): void; get(key: K): V | undefined; has(key: K): boolean; @@ -57,8 +57,8 @@ class MapLike { } forEach( callbackfn: (value: V, key: K, map: MapLike) => void, - thisArg?: any + thisArg?: any, ): void {} } -export default typeof Map === "undefined" ? Map: MapLike; +export default typeof Map === "undefined" ? Map : MapLike; From 7c78f493f44c83c47d7272cbdd3a4c7dc0f989f5 Mon Sep 17 00:00:00 2001 From: tom pridham Date: Fri, 7 Jun 2024 16:48:32 -0600 Subject: [PATCH 4/9] fix: lint errors --- sources/accessible-name-and-description.ts | 5 +++-- sources/polyfills/MapLike.ts | 10 +++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/sources/accessible-name-and-description.ts b/sources/accessible-name-and-description.ts index 2d8a9148..66d13fc0 100644 --- a/sources/accessible-name-and-description.ts +++ b/sources/accessible-name-and-description.ts @@ -362,8 +362,9 @@ export function computeTextAlternative( if (pseudoElement !== undefined) { return _getComputedStyle(el, pseudoElement); } - if (computedStyles.has(el)) { - return computedStyles.get(el)!; + const cachedStyles = computedStyles.get(el); + if (cachedStyles) { + return cachedStyles; } const style = _getComputedStyle(el, pseudoElement); computedStyles.set(el, style); diff --git a/sources/polyfills/MapLike.ts b/sources/polyfills/MapLike.ts index 099ceb90..374daf7c 100644 --- a/sources/polyfills/MapLike.ts +++ b/sources/polyfills/MapLike.ts @@ -5,7 +5,7 @@ declare global { delete(key: K): boolean; forEach( callbackfn: (value: V, key: K, map: Map) => void, - thisArg?: any, + thisArg?: this, ): void; get(key: K): V | undefined; has(key: K): boolean; @@ -57,8 +57,12 @@ class MapLike { } forEach( callbackfn: (value: V, key: K, map: MapLike) => void, - thisArg?: any, - ): void {} + thisArg?: this, + ): void { + for (let i = 0; i < this.keys.length; i++) { + callbackfn(this.values[i], this.keys[i], thisArg || this); + } + } } export default typeof Map === "undefined" ? Map : MapLike; From 4eee4b6fa2e7dc98984ffe785da47356ea18e960 Mon Sep 17 00:00:00 2001 From: tom pridham Date: Wed, 19 Jun 2024 15:50:34 -0600 Subject: [PATCH 5/9] fix: make it clear when using uncached vs cached version, dont cache if using polyfill --- sources/accessible-name-and-description.ts | 29 +++++++++++++++------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/sources/accessible-name-and-description.ts b/sources/accessible-name-and-description.ts index 66d13fc0..84ced5a4 100644 --- a/sources/accessible-name-and-description.ts +++ b/sources/accessible-name-and-description.ts @@ -351,22 +351,33 @@ export function computeTextAlternative( // window.getComputedStyle(elementFromAnotherWindow) or if I don't bind it // the type declarations don't require a `this` // eslint-disable-next-line no-restricted-properties - getComputedStyle: _getComputedStyle = window.getComputedStyle.bind(window), + getComputedStyle: uncachedGetComputedStyle = window.getComputedStyle.bind( + window, + ), hidden = false, } = options; - const cachingGetComputedStyle: GetComputedStyle = ( + const getComputedStyle: GetComputedStyle = ( el, pseudoElement, ): CSSStyleDeclaration => { - // we don't cache the pseudoElement styles + // We don't cache the pseudoElement styles and calls with psuedo elements + // should use the uncached version instead if (pseudoElement !== undefined) { - return _getComputedStyle(el, pseudoElement); + throw new Error( + "use uncachedGetComputedStyle directly for pseudo elements", + ); + } + // If Map is not available, it is probably faster to just use the uncached + // version since the polyfill lookup is O(n) instead of O(1) and + // the getComputedStyle function in those environments(e.g. IE11) is fast + if (Map === undefined) { + return uncachedGetComputedStyle(el); } const cachedStyles = computedStyles.get(el); if (cachedStyles) { return cachedStyles; } - const style = _getComputedStyle(el, pseudoElement); + const style = uncachedGetComputedStyle(el, pseudoElement); computedStyles.set(el, style); return style; }; @@ -381,7 +392,7 @@ export function computeTextAlternative( ): string { let accumulatedText = ""; if (isElement(node) && computedStyleSupportsPseudoElements) { - const pseudoBefore = cachingGetComputedStyle(node, "::before"); + const pseudoBefore = uncachedGetComputedStyle(node, "::before"); const beforeContent = getTextualContent(pseudoBefore); accumulatedText = `${beforeContent} ${accumulatedText}`; } @@ -400,14 +411,14 @@ export function computeTextAlternative( // TODO: Unclear why display affects delimiter // see https://github.com/w3c/accname/issues/3 const display = isElement(child) - ? cachingGetComputedStyle(child).getPropertyValue("display") + ? getComputedStyle(child).getPropertyValue("display") : "inline"; const separator = display !== "inline" ? " " : ""; // trailing separator for wpt tests accumulatedText += `${separator}${result}${separator}`; }); if (isElement(node) && computedStyleSupportsPseudoElements) { - const pseudoAfter = cachingGetComputedStyle(node, "::after"); + const pseudoAfter = uncachedGetComputedStyle(node, "::after"); const afterContent = getTextualContent(pseudoAfter); accumulatedText = `${accumulatedText} ${afterContent}`; } @@ -588,7 +599,7 @@ export function computeTextAlternative( // 2A if ( !hidden && - isHidden(current, cachingGetComputedStyle) && + isHidden(current, getComputedStyle) && !context.isReferenced ) { consultedNodes.add(current); From ee9005154761f7d1612a71e248287834ec79b5f0 Mon Sep 17 00:00:00 2001 From: tom pridham Date: Thu, 11 Jul 2024 11:52:14 -0600 Subject: [PATCH 6/9] fix: remove map polyfill completely so avoid accidental uses --- sources/accessible-name-and-description.ts | 10 ++-- sources/polyfills/MapLike.ts | 68 ---------------------- 2 files changed, 6 insertions(+), 72 deletions(-) delete mode 100644 sources/polyfills/MapLike.ts diff --git a/sources/accessible-name-and-description.ts b/sources/accessible-name-and-description.ts index 84ced5a4..45978990 100644 --- a/sources/accessible-name-and-description.ts +++ b/sources/accessible-name-and-description.ts @@ -3,7 +3,6 @@ */ import ArrayFrom from "./polyfills/array.from"; import SetLike from "./polyfills/SetLike"; -import MapLike from "./polyfills/MapLike"; import { hasAnyConcreteRoles, isElement, @@ -340,7 +339,10 @@ export function computeTextAlternative( options: ComputeTextAlternativeOptions = {}, ): string { const consultedNodes = new SetLike(); - const computedStyles = new MapLike(); + const computedStyles = + typeof Map === "function" + ? new Map() + : undefined; const window = safeWindow(root); const { @@ -368,9 +370,9 @@ export function computeTextAlternative( ); } // If Map is not available, it is probably faster to just use the uncached - // version since the polyfill lookup is O(n) instead of O(1) and + // version since a polyfill lookup would be O(n) instead of O(1) and // the getComputedStyle function in those environments(e.g. IE11) is fast - if (Map === undefined) { + if (computedStyles === undefined) { return uncachedGetComputedStyle(el); } const cachedStyles = computedStyles.get(el); diff --git a/sources/polyfills/MapLike.ts b/sources/polyfills/MapLike.ts deleted file mode 100644 index 374daf7c..00000000 --- a/sources/polyfills/MapLike.ts +++ /dev/null @@ -1,68 +0,0 @@ -declare global { - class Map { - // es2015.collection.d.ts - clear(): void; - delete(key: K): boolean; - forEach( - callbackfn: (value: V, key: K, map: Map) => void, - thisArg?: this, - ): void; - get(key: K): V | undefined; - has(key: K): boolean; - set(key: K, value: V): this; - readonly size: number; - } -} - -class MapLike { - private keys: K[]; - private values: V[]; - - constructor() { - this.keys = []; - this.values = []; - } - clear() { - this.keys = []; - this.values = []; - } - delete(key: K): boolean { - const index = this.keys.indexOf(key); - if (index === -1) { - return false; - } - this.keys.splice(index, 1); - this.values.splice(index, 1); - return true; - } - get(key: K): V | undefined { - const index = this.keys.indexOf(key); - if (index === -1) { - return undefined; - } - return this.values[index]; - } - has(key: K): boolean { - return this.keys.indexOf(key) !== -1; - } - set(key: K, value: V): this { - const index = this.keys.indexOf(key); - if (index === -1) { - this.keys.push(key); - this.values.push(value); - } else { - this.values[index] = value; - } - return this; - } - forEach( - callbackfn: (value: V, key: K, map: MapLike) => void, - thisArg?: this, - ): void { - for (let i = 0; i < this.keys.length; i++) { - callbackfn(this.values[i], this.keys[i], thisArg || this); - } - } -} - -export default typeof Map === "undefined" ? Map : MapLike; From b20d2d0a87a892c48609a4bd3d7efaa8c987ca0b Mon Sep 17 00:00:00 2001 From: tom pridham Date: Tue, 16 Jul 2024 12:31:50 -0600 Subject: [PATCH 7/9] fix: add Map type stub back in with note about not including an actual polyfill --- sources/accessible-name-and-description.ts | 8 ++++---- sources/polyfills/Map.ts | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 sources/polyfills/Map.ts diff --git a/sources/accessible-name-and-description.ts b/sources/accessible-name-and-description.ts index 45978990..24e953fd 100644 --- a/sources/accessible-name-and-description.ts +++ b/sources/accessible-name-and-description.ts @@ -338,13 +338,13 @@ export function computeTextAlternative( root: Element, options: ComputeTextAlternativeOptions = {}, ): string { + const window = safeWindow(root); const consultedNodes = new SetLike(); const computedStyles = - typeof Map === "function" - ? new Map() - : undefined; + typeof Map === "undefined" + ? undefined + : new Map(); - const window = safeWindow(root); const { compute = "name", computedStyleSupportsPseudoElements = options.getComputedStyle !== diff --git a/sources/polyfills/Map.ts b/sources/polyfills/Map.ts new file mode 100644 index 00000000..53b75de5 --- /dev/null +++ b/sources/polyfills/Map.ts @@ -0,0 +1,20 @@ +declare global { + class Map { + // es2015.collection.d.ts + clear(): void; + delete(key: K): boolean; + forEach( + callbackfn: (value: V, key: K, map: Map) => void, + thisArg?: unknown, + ): void; + get(key: K): V | undefined; + has(key: K): boolean; + set(key: K, value: V): this; + readonly size: number; + } +} + +// we need to export something here to make this file a module, but don't want to +// actually include a polyfill because it's potentially significantly slower than +// the native implementation +export {}; From f2b6eb5079f89b2a51e72d00f4d8544c0cf09fa2 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 17 Jul 2024 15:10:00 +0200 Subject: [PATCH 8/9] clean git diff --- sources/accessible-name-and-description.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/sources/accessible-name-and-description.ts b/sources/accessible-name-and-description.ts index 24e953fd..7ccbec17 100644 --- a/sources/accessible-name-and-description.ts +++ b/sources/accessible-name-and-description.ts @@ -338,13 +338,13 @@ export function computeTextAlternative( root: Element, options: ComputeTextAlternativeOptions = {}, ): string { - const window = safeWindow(root); const consultedNodes = new SetLike(); const computedStyles = typeof Map === "undefined" ? undefined : new Map(); + const window = safeWindow(root); const { compute = "name", computedStyleSupportsPseudoElements = options.getComputedStyle !== @@ -387,10 +387,7 @@ export function computeTextAlternative( // 2F.i function computeMiscTextAlternative( node: Node, - context: { - isEmbeddedInLabel: boolean; - isReferenced: boolean; - }, + context: { isEmbeddedInLabel: boolean; isReferenced: boolean }, ): string { let accumulatedText = ""; if (isElement(node) && computedStyleSupportsPseudoElements) { From 109ccbd9387c8812dc6addd6b139627dd1eedf16 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 17 Jul 2024 15:16:30 +0200 Subject: [PATCH 9/9] Add changesetr --- .changeset/spotty-chicken-add.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .changeset/spotty-chicken-add.md diff --git a/.changeset/spotty-chicken-add.md b/.changeset/spotty-chicken-add.md new file mode 100644 index 00000000..73e43a89 --- /dev/null +++ b/.changeset/spotty-chicken-add.md @@ -0,0 +1,9 @@ +--- +"dom-accessibility-api": minor +--- + +Cache `window.getComputedStyle` results + +Should improve performance in environments that don't cache these results natively e.g. JSDOM. +This increases memory usage. +If this results in adverse effects (e.g. resource constrained browser environments), please file an issue.