Siteimprove / alfa

:wheelchair: Suite of open and standards-based tools for performing reliable accessibility conformance testing at scale
MIT License
102 stars 11 forks source link

Add verbosity option for JSON serialization of tree nodes #1618

Closed rcj-siteimprove closed 1 month ago

rcj-siteimprove commented 1 month ago

We introduce a verbosity option for JSON serialization of tree nodes in general and in particular for DOM tree nodes. The purpose is to avoid redundant serialization of the same nodes over and over again when they are referenced multiple times. This is for instance the case in the result of an audit where the same DOM node might be the target of many different rules. We enable this by generating a random UUID on construction of each DOM node, which we call a serializationId.

Serializing with Minimal or Low verbosity will then only output the serializationId and the type.

Serializing with Medium verbosity or not supplying a verbosity will result in an output containing all properties except the serializationId - the same as the output before verbosity was introduced. This makes the change backwards compatible as doing nothing will get you the same result as before.

Serializing with High verbosity will result in an output with all properties and the serializationId. This option is meant to be used for, and is necessary for, establishing the map from ID to fully serialized DOM node. It's the callers responsibility to do this and to look up the nodes by ID when the full object needs to be reconstructed from the serializationId.

Additionally, we now depend on crypto to be present globally, meaning that we are no longer compatible with Node 18.

changeset-bot[bot] commented 1 month ago

đŸ¦‹ Changeset detected

Latest commit: 15e945477834fbbe0c1f2b06eaec0584cacd3675

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 78 packages | Name | Type | | ------------------------------- | ----- | | @siteimprove/alfa-dom | Minor | | @siteimprove/alfa-rules | Minor | | @siteimprove/alfa-json | Minor | | @siteimprove/alfa-tree | Minor | | @siteimprove/alfa-act | Minor | | @siteimprove/alfa-compatibility | Minor | | @siteimprove/alfa-continuation | Minor | | @siteimprove/alfa-applicative | Minor | | @siteimprove/alfa-css-feature | Minor | | @siteimprove/alfa-performance | Minor | | @siteimprove/alfa-collection | Minor | | @siteimprove/alfa-comparable | Minor | | @siteimprove/alfa-refinement | Minor | | @siteimprove/alfa-trampoline | Minor | | @siteimprove/alfa-equatable | Minor | | @siteimprove/alfa-generator | Minor | | @siteimprove/alfa-predicate | Minor | | @siteimprove/alfa-rectangle | Minor | | @siteimprove/alfa-selective | Minor | | @siteimprove/alfa-toolchain | Minor | | @siteimprove/alfa-branched | Minor | | @siteimprove/alfa-callback | Minor | | @siteimprove/alfa-encoding | Minor | | @siteimprove/alfa-foldable | Minor | | @siteimprove/alfa-iterable | Minor | | @siteimprove/alfa-selector | Minor | | @siteimprove/alfa-sequence | Minor | | @siteimprove/alfa-thenable | Minor | | @siteimprove/alfa-cascade | Minor | | @siteimprove/alfa-emitter | Minor | | @siteimprove/alfa-functor | Minor | | @siteimprove/alfa-json-ld | Minor | | @siteimprove/alfa-network | Minor | | @siteimprove/alfa-promise | Minor | | @siteimprove/alfa-reducer | Minor | | @siteimprove/alfa-trilean | Minor | | @siteimprove/alfa-affine | Minor | | @siteimprove/alfa-device | Minor | | @siteimprove/alfa-either | Minor | | @siteimprove/alfa-future | Minor | | @siteimprove/alfa-mapper | Minor | | @siteimprove/alfa-option | Minor | | @siteimprove/alfa-parser | Minor | | @siteimprove/alfa-record | Minor | | @siteimprove/alfa-result | Minor | | @siteimprove/alfa-string | Minor | | @siteimprove/alfa-array | Minor | | @siteimprove/alfa-cache | Minor | | @siteimprove/alfa-clone | Minor | | @siteimprove/alfa-flags | Minor | | @siteimprove/alfa-graph | Minor | | @siteimprove/alfa-media | Minor | | @siteimprove/alfa-monad | Minor | | @siteimprove/alfa-sarif | Minor | | @siteimprove/alfa-slice | Minor | | @siteimprove/alfa-style | Minor | | @siteimprove/alfa-table | Minor | | @siteimprove/alfa-thunk | Minor | | @siteimprove/alfa-tuple | Minor | | @siteimprove/alfa-xpath | Minor | | @siteimprove/alfa-aria | Minor | | @siteimprove/alfa-bits | Minor | | @siteimprove/alfa-earl | Minor | | @siteimprove/alfa-hash | Minor | | @siteimprove/alfa-http | Minor | | @siteimprove/alfa-iana | Minor | | @siteimprove/alfa-lazy | Minor | | @siteimprove/alfa-list | Minor | | @siteimprove/alfa-math | Minor | | @siteimprove/alfa-test | Minor | | @siteimprove/alfa-time | Minor | | @siteimprove/alfa-wcag | Minor | | @siteimprove/alfa-css | Minor | | @siteimprove/alfa-fnv | Minor | | @siteimprove/alfa-map | Minor | | @siteimprove/alfa-set | Minor | | @siteimprove/alfa-url | Minor | | @siteimprove/alfa-web | Minor |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

rcj-siteimprove commented 1 month ago

!pr extract

rcj-siteimprove commented 1 month ago

@Jym77, apparently crypto is not available as a global variable in Node 18, only after 19 making the tests fail for that version and not the others. It's possible to run the node 18 tests with the flag --experimental-global-webcrypto, but I'm not sure if that is something we want to do. Otherwise we need to import it everywhere we use it. Not sure if that will break something else. What do you think we should do?

Edit: the flag mentioned doesn't seem to work anyway, so it seems we need to import crypto.

Jym77 commented 1 month ago

@Jym77, apparently crypto is not available as a global variable in Node 18, only after 19 making the tests fail for that version and not the others. ~It's possible to run the node 18 tests with the flag --experimental-global-webcrypto, but I'm not sure if that is something we want to do~. Otherwise we need to import it everywhere we use it. Not sure if that will break something else. What do you think we should do?

Edit: the flag mentioned doesn't seem to work anyway, so it seems we need to import crypto.

18 is already in LTS mode: https://nodejs.org/en/about/previous-releases

Maybe we can just move on and switch to supporting 20+ (update the actions to use 20, 22 instead of 18, 20). This does require a breaking changeset, though…

rcj-siteimprove commented 1 month ago

!pr extract