fix(d.ts): RunOptions.reporter can be any string #4218
Conversation
| @@ -132,7 +132,7 @@ declare namespace axe { | |||
| interface RunOptions { | |||
| runOnly?: RunOnly | TagValue[] | string[] | string; | |||
| rules?: RuleObject; | |||
| reporter?: ReporterVersion; | |||
| reporter?: ReporterVersion | string; | |||
There was a problem hiding this comment.
I don't think this adds any value in typescript. The resolved type of this is just string and it looks like it overrides any value of the ReporterVersion. Might just make this string and not have the set values.
There was a problem hiding this comment.
Sure, but can we do better? I looked at using a generic for this, something like:
interface RunOptions <T extends string | void = void>{
reporter?: ReporterVersion | T;
}That's fine, but then you can't pass this into axe.run because it expects RunOptions<void>. To fix that you'd have to make the reporter something that is passed into axe.run, and to AxeResults (for toolOptions), and AxeReporter, and getFrameContexts... It's a lot. I don't think that's worth the complexity. If someone want to be sure their reporter type is valid they can still check it with reporterVersion. Open to suggestions though.
There was a problem hiding this comment.
Looking at a GitHub-wide code search for "axe.addReporter", there are exactly zero folks who are actually using axe.addReporter in practice in an open source project (all hits are copies of axe-core or its tests).
Based on that, I definitely don't think making this more accurate is worth making the entire RunOptions type much more complex, and I don't think it's worth making the intellisense worse (like steve's screenshots point out) either; my inclination is to optimize for folks that aren't using addReporter and actually just do this:
/** If you've added a custom ReporterVersion with `axe.addReporter`, that is also valid here */
reporter?: ReporterVersionIn practice, I think if we're improving this type, the more important improvement would be to add the existing AxeReporter function as an allowable value.
There was a problem hiding this comment.
(I can also live with ReporterVersion | string if Wilco feels strongly about it - I only feel strongly that we shouldn't pollute the whole RunOptions type with a generic over this)
There was a problem hiding this comment.
I do prefer ReporterVersion | string here.
Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>
## [4.9.0](v4.8.4...v4.9.0) (2024-03-25) ### Features - adding the wcag131 tag to the aria-hidden-body rule ([#4349](#4349)) ([dd4c3c3](dd4c3c3)), closes [#4315](#4315) - **checks:** deprecate aria-busy check ([#4356](#4356)) ([be0b555](be0b555)), closes [#4347](#4347) [#4340](#4340) - **color:** add color channel values and luminosity, saturation, clip functions ([#4366](#4366)) ([9e70199](9e70199)), closes [/github.com//pull/4365/files#r1517706612](https://github.com/dequelabs//github.com/dequelabs/axe-core/pull/4365/files/issues/r1517706612) - **i18n:** add Greek Translations ([#3836](#3836)) ([3ea9a48](3ea9a48)) - **i18n:** Add Italian translation ([#4344](#4344)) ([de1baa9](de1baa9)) - **i18n:** Add Simplified Chinese translation ([#4379](#4379)) ([bda7c8d](bda7c8d)) - **i18n:** Add Taiwanese Mandarin translation ([#4299](#4299)) ([c5e11de](c5e11de)) ### Bug Fixes - Add LICENSE-3RD-PARTY.txt file ([#4304](#4304)) ([daa0fe6](daa0fe6)) - add Object.values polyfill for node <=6 ([#4274](#4274)) ([5eb867b](5eb867b)) - **aria-required-children:** avoid confusing aria-busy message in failures ([#4347](#4347)) ([591607d](591607d)), closes [#fail13](https://github.com/dequelabs/axe-core/issues/fail13) [#4340](#4340) - avoid reading element-specific node properties of non-element node types ([#4317](#4317)) ([b853b18](b853b18)), closes [#4316](#4316) [#4316](#4316) - **color-contrast:** handle text that is outside `overflow: hidden` ancestor ([#4357](#4357)) ([bdb7300](bdb7300)), closes [#4253](#4253) - **color-contrast:** support color blend modes hue, saturation, color, luminosity ([#4365](#4365)) ([7ae4761](7ae4761)) - **d.ts:** RawNodesResult issues ([#4229](#4229)) ([d660518](d660518)) - **d.ts:** RunOptions.reporter can be any string ([#4218](#4218)) ([e53f5c5](e53f5c5)) - **i18n:** update Italian translations ([#4377](#4377)) ([4d65d4b](4d65d4b)) - **listitem:** clarify roleNotValid message ([#4374](#4374)) ([0f8a9af](0f8a9af)) - **scrollable-region-focusable:** missing wcag213 tag ([#4201](#4201)) ([0080a72](0080a72)) - **target-size:** always pass 10x targets (avoid perf bottleneck) ([#4376](#4376)) ([be327c4](be327c4)) - **target-size:** do not crash for nodes with many overlapping widgets ([#4373](#4373)) ([1dbea83](1dbea83)), closes [#4359](#4359) [#4359](#4359) [#4360](#4360) - **utils/get-selector:** ignore 'xmlns' attribute when generating a selector ([#4303](#4303)) ([938b411](938b411)) This PR was opened by a robot 🤖 🎉
raw-envis not a valid reporter.Closes: #4217