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

Propagate serialization options in diagnostics #1638

Closed rcj-siteimprove closed 2 days ago

rcj-siteimprove commented 1 week ago

Propagate the serialization options in Diagnostic subclasses so that they are eventually applied when serializing Elements.

I have deliberately only propagated the options where they would eventually be passed to Element.toJSON. Propagating them to all sub-calls would explode and the options wouldn't be used for anything anyway, so it seems somewhat a waste of time.

changeset-bot[bot] commented 1 week ago

🦋 Changeset detected

Latest commit: d6f056d835c5d79048364f0e3ea532bf9330968a

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

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

!pr extract

Jym77 commented 3 days ago

Let's go a bit back on the changes. Instead of adding the type parameter everywhere, we can keep it on the toJSON() method. Also need to keep sure all properties are optionals.

class Diagnostic {
  …
  public toJSON(options: json.SerializationOptions) { … }
}

class DiagWithNodeAndFooAndBar {
  …
  public toJSON(options: Node.SerializationOptions & Foo.SerializationOptions & Bar.SerializationOptions) { … }

This does, however require we also change the existing ones for consistency.

rcj-siteimprove commented 3 days ago

!pr extract

rcj-siteimprove commented 3 days ago

Let's go a bit back on the changes. Instead of adding the type parameter everywhere, we can keep it on the toJSON() method. Also need to keep sure all properties are optionals.

class Diagnostic {
  …
  public toJSON(options: json.SerializationOptions) { … }
}

class DiagWithNodeAndFooAndBar {
  …
  public toJSON(options: Node.SerializationOptions & Foo.SerializationOptions & Bar.SerializationOptions) { … }

This does, however require we also change the existing ones for consistency.

@Jym77 This should be done now

rcj-siteimprove commented 3 days ago

!pr extract