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(d.ts): improve axe.d.ts types #4081

Merged
merged 7 commits into from Jul 19, 2023
Merged

Conversation

hamax
Copy link
Contributor

@hamax hamax commented Jul 5, 2023

Improve axe.d.ts types based on what we found was missing/incorrect and was needed in our code. I added more details for each change in commit messages (I tried to verify that each change is correct by looking at axe internals). Some of the issues were introduced with a recent update in #3966, which prompted me to look into this more.

hamax added 3 commits July 5, 2023 17:26
Before shadow dom support string[] was the correct type for the
returned selectors. From code we can see that SerialDqElement.selector
is used to populate this, so the type should also be the same.
RelatedNode should have the same selectors and the same element
reference as NodeResult. These are populated in process-aggregate.js.
I think this was unintentionally changed in dequelabs#3966. From check
definitions we can see that many of them don't specify the incomplete
message, because they don't have incomplete as a possible result.
@hamax hamax requested a review from a team as a code owner July 5, 2023 18:19
@WilcoFiers WilcoFiers self-assigned this Jul 10, 2023
Copy link
Contributor

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening this PR. I am really curious what you are using these methods for.

axe.d.ts Outdated Show resolved Hide resolved
axe.d.ts Outdated Show resolved Hide resolved
axe.d.ts Outdated Show resolved Hide resolved
axe.d.ts Outdated Show resolved Hide resolved
axe.d.ts Outdated Show resolved Hide resolved
axe.d.ts Outdated Show resolved Hide resolved
More precise types make it easier to write these function in typescript.
With this change, complier knows about this.data, this.async, etc.
Like evaluate and after, matches can also be a string or a function, but
this is not currently represented in types.
The rule metadata object expected in this.configure and the one returned
by getRules are not the same. From the code we can see that tags and
actIds are read from the rule itself and not from the metadata object.
@hamax
Copy link
Contributor Author

hamax commented Jul 18, 2023

Thanks, I amended the commits with suggestions (+ prettier formatting).

Thanks for opening this PR. I am really curious what you are using these methods for.

Mostly for custom rules/checks and capturing additional debug information for nodes in results. I think it makes sense for utils commonly used from checks to be available.

@WilcoFiers
Copy link
Contributor

✅ Reviewed for security.

@WilcoFiers WilcoFiers merged commit 7c5f991 into dequelabs:develop Jul 19, 2023
16 of 17 checks passed
@WilcoFiers
Copy link
Contributor

Thank you again for your contribution @hamax!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants