A11y: Add types #19096
A11y: Add types #19096
Conversation
sirreal added [Type] Code Quality
packages/a11y/src/addContainer.js Outdated
| document.querySelector( 'body' ).appendChild( container ); | ||
| // Disable reason: the querySelector + optional chain may make the expression `null`. | ||
| // eslint-disable-next-line no-unused-expressions | ||
| document.querySelector( 'body' )?.appendChild( container ); |
There was a problem hiding this comment.
This may be enough of an edge case that we could use a JSDoc cast:
| document.querySelector( 'body' )?.appendChild( container ); | |
| /** @type { HTMLBodyElement } */ ( document.querySelector( 'body' ) ).appendChild( container ); |
The optional chain does seem to be correct, it's possible to have a document with no body:
document.createDocumentFragment().querySelector( 'body' ).appendChild( document.createElement( 'div' ) )
// Uncaught TypeError: Cannot read property 'appendChild' of null
That is an extreme edge case, however 🙂
There was a problem hiding this comment.
Whoops, I thought optional chaining was available already.
This change is blocked on #18939 or an alternative implementation.
| } | ||
| if ( containerAssertive === null ) { | ||
| containerAssertive = addContainer( 'assertive' ); | ||
| addContainer( 'assertive' ); |
There was a problem hiding this comment.
I removed the reassignment of the local variables which seems to serve no purpose here.
sirreal force-pushed the add/types-a11y branch from 8f27e9d to 992c5e9 Compare
Rebased and feedback addressed. |
sirreal force-pushed the add/types-a11y branch from 992c5e9 to 6efcba4 Compare
This has some an issue with tsc runs: We'll need to figure out how to make tsc aware of other packages, likely along the lines of #18838 (comment) |
sirreal force-pushed the add/types-a11y branch from 6efcba4 to c87f3e2 Compare
tsconfig.json Outdated
| "baseUrl": ".", | ||
| "paths": { | ||
| "@wordpress/*": [ "./packages/*/src" ] | ||
| } |
There was a problem hiding this comment.
This fixes the issues with the unfound @wordpress/dom-ready module and doesn't seem to introduce any issues.
Seems sufficient for now.
There was a problem hiding this comment.
This fixes the issues with the unfound
@wordpress/dom-readymodule and doesn't seem to introduce any issues.Seems sufficient for now.
Related: #18927
Once this is in place, we'll probably have trouble adding types to anything which depends on another package which isn't type-ready (i.e. if there are any errors in the dependency code).
There was a problem hiding this comment.
we'll probably have trouble adding types to anything which depends on another package which isn't type-ready
Based on my experience here, it's already a problem because tsc can't locate the modules.
There was a problem hiding this comment.
Would you prefer to sort that out in #18927 or shall we include it here and merge this?
sirreal force-pushed the add/types-a11y branch from 12072f3 to 8003ce6 Compare
sirreal force-pushed the add/types-a11y branch from 8003ce6 to 5eed3ea Compare
I've rebased, this should be ready to land now. |
Description
Part of #18838
Extracted from #18942
Add types to
a11ypackage.Testing Instructions:
Verify type checking passes: