Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: reduce transpilation helpers #240

Merged
merged 16 commits into from
May 16, 2020
Merged
12 changes: 12 additions & 0 deletions .changeset/poor-ways-film.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"dom-accessibility-api": patch
---

Reduce over-transpilation

Switched from

- for-of to `.forEach` or basic for loop
- `array.push(...otherArray)` to `push.apply(array, otherArray)`

This removed a bunch of babel junk that wasn't needed.
18 changes: 16 additions & 2 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,18 @@
// own implementation. See https://github.com/eps1lon/dom-accessibility-api/blob/eb868428a31a093aecc531bf2dd17e8547bd0c3b/sources/accessible-name.ts#L33
"property": "getComputedStyle"
}
]
],
"no-restricted-syntax": [
"error",
{
"selector": "ForOfStatement",
"message": "for-of assumes iterators which require heavy transpilation for es5. Use ArrayFrom(...).forEach instead."
}
],
// Babel transpiles array spread assuming iterators.
// If we know we have an array we cann use .apply instead.
// TypeScript will validate that claim.
"prefer-spread": "off"
},
"overrides": [
{
Expand All @@ -48,7 +59,10 @@
// disable previous window.getComputedStyles restriction
// perf concerns for this aren't applicable to test
// since we specifically want to test them
"no-restricted-properties": "off"
"no-restricted-properties": "off",
// disable previous for-of restriction
// over transpilation isn't a concern for tests
"no-restricted-syntax": "off"
}
}
]
Expand Down
25 changes: 24 additions & 1 deletion azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,34 @@ steps:
- script: |
npm pack
mv dom-accessibility-api-*.tgz dom-accessibility-api.tgz
displayName: "Prepare smoke tests of build"
displayName: "Create tarball"

- publish: $(System.DefaultWorkingDirectory)/dom-accessibility-api.tgz
displayName: "Publish tarball"
artifact: dom-accessibility-api-node-$(node_version)

- task: DownloadPipelineArtifact@2
displayName: "Download tarball from master"
inputs:
artifact: dom-accessibility-api-node-$(node_version)
path: $(Agent.TempDirectory)/artifacts-master
source: specific
pipeline: $(System.DefinitionId)
project: $(System.TeamProject)
runVersion: latestFromBranch
runBranch: refs/heads/master

- script: |
mkdir $(Agent.TempDirectory)/published-previous
mkdir $(Agent.TempDirectory)/published-current
tar xfz $(Agent.TempDirectory)/artifacts-master/dom-accessibility-api.tgz --directory $(Agent.TempDirectory)/published-previous
tar xfz $(System.DefaultWorkingDirectory)/dom-accessibility-api.tgz --directory $(Agent.TempDirectory)/published-current
# --no-index implies --exit-code
# This task is informative only.
# Diffs are almost always expected
git --no-pager diff --color --no-index $(Agent.TempDirectory)/published-previous $(Agent.TempDirectory)/published-current || exit 0
displayName: "Diff tarballs"

- script: yarn start
displayName: "kcd-rollup smoke tests of build"
workingDirectory: tests/build/fixtures/kcd-rollup
Expand Down
17 changes: 10 additions & 7 deletions sources/accessible-name.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,12 @@ function querySelectorAllSubtree(
element: Element,
selectors: string
): Element[] {
const elements = [];
const elements = ArrayFrom(element.querySelectorAll(selectors));

for (const root of [element, ...idRefs(element, "aria-owns")]) {
elements.push(...ArrayFrom(root.querySelectorAll(selectors)));
}
idRefs(element, "aria-owns").forEach((root) => {
// babel transpiles this assuming an iterator
elements.push.apply(elements, ArrayFrom(root.querySelectorAll(selectors)));
});

return elements;
}
Expand Down Expand Up @@ -314,7 +315,7 @@ export function computeAccessibleName(
const childNodes = ArrayFrom(node.childNodes).concat(
idRefs(node, "aria-owns")
);
for (const child of childNodes) {
childNodes.forEach((child) => {
const result = computeTextAlternative(child, {
isEmbeddedInLabel: context.isEmbeddedInLabel,
isReferenced: false,
Expand All @@ -326,7 +327,7 @@ export function computeAccessibleName(
createGetComputedStyle(node, options)(node).getPropertyValue("display");
const separator = display !== "inline" ? " " : "";
accumulatedText += `${separator}${result}`;
}
});

if (isElement(node)) {
const pseudoAfter = createGetComputedStyle(node, options)(node, ":after");
Expand Down Expand Up @@ -366,7 +367,9 @@ export function computeAccessibleName(
// https://w3c.github.io/html-aam/#fieldset-and-legend-elements
if (isHTMLFieldSetElement(node)) {
consultedNodes.add(node);
for (const child of ArrayFrom(node.childNodes)) {
const children = ArrayFrom(node.childNodes);
for (let i = 0; i < children.length; i += 1) {
const child = children[i];
if (isHTMLLegendElement(child)) {
return computeTextAlternative(child, {
isEmbeddedInLabel: false,
Expand Down