Page MenuHomePhabricator

Resolve Element vs HTMLElement when using TypeScript checking
Closed, ResolvedPublic3 Estimated Story Points

Description

Vector uses tsc to check code for type safety. Whenever we have an Element, we almost certainly have an HTMLElement. As various properties only exist on HTMLElement (see https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement) - we need to make sure elements are typed as HTMLElement and not just Element, however many methods return the Element interface instead - most notably querySelector/querySelectorAll.

I think there are two options:

  1. Add an inline typehint every time we use an Element-returning method:
const foo = /** @type {HTMLElement} */ container.querySelector( '.foo' ),
    bar = /** @type {HTMLElement} */ container.querySelector( '.bar' ),
  1. Create utility functions:
/**
 * @param {string} selector
 * @param {HTMLElement} [parent=document]
 * @returns {HTMLElement|null}
 */
function querySelector( selector, parent = document ) {
    return parent.querySelector( selector );
}

Event Timeline

There are already some similar workarounds in place in the codebase, e.g. where we get a Node but need an Element:

const
	// Type declaration needed because of https://github.com/Microsoft/TypeScript/issues/3734#issuecomment-118934518
	userLinksDropdownClone = /** @type {Element} */( userLinksDropdown.cloneNode( true ) ),
	userLinksDropdownStickyElementsWithIds = userLinksDropdownClone.querySelectorAll( '[ id ], [ data-event-name ]' );

Change #1193090 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/skins/Vector@master] Fix methods which return {Element} to return {HTMLElement}

https://gerrit.wikimedia.org/r/1193090

Change #1193097 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/skins/Vector@master] stickyHeader.js: Replace {Element} types with {HTMLElement}

https://gerrit.wikimedia.org/r/1193097

Change #1193103 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/skins/Vector@master] Replace all remaining {Element} types with {HTMLElement}

https://gerrit.wikimedia.org/r/1193103

Change #1193090 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Fix methods which return {Element} to return {HTMLElement}

https://gerrit.wikimedia.org/r/1193090

Change #1193097 merged by jenkins-bot:

[mediawiki/skins/Vector@master] stickyHeader.js: Replace {Element} types with {HTMLElement}

https://gerrit.wikimedia.org/r/1193097

Change #1193103 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Replace all remaining {Element} types with {HTMLElement}

https://gerrit.wikimedia.org/r/1193103