Skip to content

feat(axe.d.ts): add nodeSerializer typings - #4551

Merged
dbjorge merged 3 commits into
dequelabs:developfrom
hamax:node-serializer
Sep 24, 2024
Merged

feat(axe.d.ts): add nodeSerializer typings#4551
dbjorge merged 3 commits into
dequelabs:developfrom
hamax:node-serializer

Conversation

@hamax

@hamax hamax commented Aug 1, 2024

Copy link
Copy Markdown
Contributor

This API was added in pr #4093, but TS definitions were never added.

For simplicity I'm using SerialDqElement in the API. We could introduce a generic for the custom serialized type (T extends SerialDqElement), but it's hard to consistently use it everywhere (AxeReporter, NodeSerializer.dqElmToSpec).

I also fixed DqElement.mergeSpecs which is needed to implement a custom node serializer: it exists on the constructor and not on the individual instances

@hamax
hamax requested a review from a team as a code owner August 1, 2024 14:57

@straker straker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry for the late response and thanks for putting this together. I have a minor suggestion.

Comment thread axe.d.ts Outdated
boundingClientRect: DOMRect;
}

interface CustomNodeSerializer {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it would be more useful to have this be a type parameter and have all functions take and return that type. It would ensure that the functions take and return the same thing.

interface CustomNodeSerializer<T = SerialDqElement> {
  toSpec: (dqElm: DqElement) => T
  mergeSpecs(nodeSpec: T, parentFrameSpec: T) => T
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I also needed to make the nodeSerializaer.update generic to be able to take any CustomNodeSerializer (if it only takes CustomNodeSerializer then making CustomNodeSerializer generic wouldn't really change anything.

There are still a lot of other functions that are not properly using this new type T here, like dqElmToSpec, RawNodeResult etc. Without changing the API, a lot of these functions would need to be return only generics, which is not great.

The way we use this is our code is by making our custom type extend SerialDqElement and only add optional additional properties which is not perfect but works around the type issue.

@hamax

hamax commented Aug 27, 2024

Copy link
Copy Markdown
Contributor Author

@straker Hey, can you take another look? See inline comment above.

@straker

straker commented Aug 29, 2024

Copy link
Copy Markdown
Contributor

Apologies, didn't see the reply you gave earlier. I'm currently working on fixing our broken unit tests and then I can take another look at this.

@dbjorge dbjorge left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for your patience and for the contribution!

@dbjorge
dbjorge dismissed straker’s stale review September 24, 2024 16:33

discussed with straker

@dbjorge

dbjorge commented Sep 24, 2024

Copy link
Copy Markdown
Contributor

Approved for security and validated formatting

@dbjorge
dbjorge merged commit a2f3a48 into dequelabs:develop Sep 24, 2024
@dbjorge dbjorge changed the title feat(axe.d.ts): add nodeSerializer API feat(axe.d.ts): add nodeSerializer typings Sep 24, 2024
@hamax
hamax deleted the node-serializer branch October 3, 2024 16:12
straker added a commit that referenced this pull request Oct 9, 2025
##
[4.11.0](v4.10.3...v4.11.0)
(2025-10-07)

### Features

- add RGAA tags to rules
([#4862](#4862))
([53a925a](53a925a))
- **aria-prohibited-attr:** add support for fallback roles
([#4325](#4325))
([62a19a9](62a19a9))
- **axe.d.ts:** add nodeSerializer typings
([#4551](#4551))
([a2f3a48](a2f3a48)),
closes [#4093](#4093)
- **DqElement:** deprecate fromFrame function
([#4881](#4881))
([374c376](374c376)),
closes [#4093](#4093)
- **DqElement:** Truncate large `html` strings when the element has a
large outerHTML string
([#4796](#4796))
([404a4fb](404a4fb)),
closes [#4544](#4544)
- **get-xpath:** return proper relative selector for id
([#4846](#4846))
([1035f9e](1035f9e)),
closes [#4845](#4845)
- **i18n:** Add Portugal Portuguese translation
([#4725](#4725))
([5b6a65a](5b6a65a))
- incomplete with node on which an error occurred
([#4863](#4863))
([32ed8da](32ed8da))
- **locale:** Added ru locale
([#4565](#4565))
([067b01d](067b01d))
- **tap:** some best practice rules map to RGAA
([#4895](#4895))
([bc33f4c](bc33f4c))
- **td-headers-attr:** report headers attribute referencing other <td>
elements as unsupported
([#4589](#4589))
([ec7c6c8](ec7c6c8)),
closes [#3987](#3987)

### Bug Fixes

- **aria-allowed-role:** add form to allowed roles of form element
([#4588](#4588))
([8aa47ac](8aa47ac)),
closes
[/github.com/dequelabs/axe-core/blob/develop/lib/standards/html-elms.js#L264](https://github.com/dequelabs//github.com/dequelabs/axe-core/blob/develop/lib/standards/html-elms.js/issues/L264)
- **aria-allowed-role:** Add math to allowed roles for img element
([#4658](#4658))
([95b6c18](95b6c18)),
closes [#4657](#4657)
- **autocomplete-valid :** Ignore readonly autocomplete field
([#4721](#4721))
([491f4ec](491f4ec)),
closes [#4708](#4708)
- **autocomplete-valid:** treat values "xon" and "xoff" as
non-WCAG-violations
([#4878](#4878))
([52bc611](52bc611)),
closes [#4877](#4877)
- **axe.d.ts:** add typings for preload options object
([#4543](#4543))
([cfd2974](cfd2974))
- **button-name,input-button-name,input-img-alt:** allow label to give
accessible name
([#4607](#4607))
([a9710d7](a9710d7)),
closes [#4472](#4472)
[#3696](#3696)
[#3696](#3696)
- **captions:** fix grammar in captions check incomplete message
([#4661](#4661))
([11de515](11de515))
- **color-contrast:** do not run on elements with font-size: 0
([#4822](#4822))
([d77c885](d77c885)),
closes [#4820](#4820)
- consistently parse tabindex, following HTML 5 spec
([#4637](#4637))
([645a850](645a850)),
closes [#4632](#4632)
- **core:** measure perf for async checks
([#4609](#4609))
([7e9bacf](7e9bacf))
- fix grammar when using "alternative text" in a sentence
([#4811](#4811))
([237a586](237a586)),
closes [#4394](#4394)
- **get-ancestry:** add nth-child selector for multiple siblings of
shadow root ([#4606](#4606))
([1cdd6c3](1cdd6c3)),
closes [#4563](#4563)
- **get-ancestry:** don't error when there is no parent
([#4617](#4617))
([a005703](a005703))
- **locale:** fix typos in japanese (ja) locale
([#4856](#4856))
([3462cc5](3462cc5))
- **locale:** fixed typos in german (DE) locale
([#4631](#4631))
([b7736de](b7736de))
- **locale:** proofread and updated de.json
([#4643](#4643))
([8060ada](8060ada))
- **meta-viewport:** lower impact to moderate
([#4887](#4887))
([2f32aa5](2f32aa5)),
closes [#4714](#4714)
- **no-autoplay-audio:** don't timeout for preload=none media elements
([#4684](#4684))
([cdc871e](cdc871e))
- **performanceTimer:** throwing in axe catch clause
([#4852](#4852))
([a4ade04](a4ade04)),
closes
[/github.com/dequelabs/axe-core/blob/e7dae4ec48cbfef74de9f833fdcfb178c1002985/lib/core/base/rule.js#L297-L300](https://github.com/dequelabs//github.com/dequelabs/axe-core/blob/e7dae4ec48cbfef74de9f833fdcfb178c1002985/lib/core/base/rule.js/issues/L297-L300)
- **performanceTimer:** work in frames
([#4834](#4834))
([d7dfebc](d7dfebc))
- **rules:** Change "alternate text" to "alternative text"
([#4582](#4582))
([b03ada3](b03ada3))
- **target-size:** do not treat focusable tabpanels as targets
([#4702](#4702))
([60d11f2](60d11f2)),
closes [#4421](#4421)
[#4701](#4701)
- **type:** correct RuleError type
([#4893](#4893))
([d1aa8e2](d1aa8e2))
- **types:** correct raw types
([#4903](#4903))
([3eade11](3eade11))

This PR was opened by a robot 🤖 🎉
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.

3 participants