Typescript: Node.parentElement should be Element, not HTMLElement

[cmlenz]

The declaration of the parentElement property on the Node interface in lib.d.ts states that it is of type HTMLElement while it should be Element (see the spec).

[zhengbli]

This is arguable, because there was complainant before saying that it was too cumbersome to always cast the type Element to HTMLElement when in most common cases the actual type is HTMLElement, even though the spec says it should be Element.

For example, the return type of getElementById is defined as Element, however we made it HTMLElement to avoid too much casting. If the type is some other Element, you can just cast it to Element first and then cast it again to the final type. I think in most cases the parentElement is HTMLElement too, therefore it might be better just leave it the way it is.

[leonyu]

I just encountered this bug. The issue occurred with SVG on the page.

Maybe this can be solved with generics? Though the syntax would be ridiculous. (e.g. Node<Element<HTMLElement>>>

[FranklinWhale]

How about changing Node.parentElement to return Element and overriding parentElement in HTMLElement to return HTMLElement?

[mhegazy]

sounds reasonable.

[FranklinWhale]

I plan to submit a PR that contains the following changes:

• documentElement and getElementById(elementId: string) in Document will return Element instead of HTMLElement.
• There will be a new interface, DocumentOf<TNamesapce, TElement, TDocumentElement>:

interface DocumentOf<TNamesapce extends string, TElement extends Element, TDocumentElement extends TElement> extends Document {
    documentElement: TDocumentElement;
    getElementById(elementId: string): TElement;
    getElementsByTagNameNS(namespaceURI: TNamesapce, localName: string): NodeListOf<TElement>;
}

• There will be a type alias, HTMLNamespace:

type HTMLNamespace = "http://www.w3.org/1999/xhtml";

• HTMLDocument will be changed to extend DocumentOf<HTMLNamespace, HTMLElement, HTMLHtmlElement>.
• The type of document in Window will be changed from Document to HTMLDocument.
• parentElement in Node will return Element, which will be overridden by HTMLElement to return HTMLElement.
• There will be a new interface, HTMLElementAttr, that extends Attr. ownerElement and parentElement in HTMLElementAttr return HTMLElement.
• getAttributeNode(name: string) in HTMLElement will return HTMLElementAttr.

Any thoughts?

[mhegazy]

note sure if you need the additional type DocumentOf. all you need is to redefine the two methods in HTMLDocument.
otherwise seems reasonable.

[FranklinWhale]

The original idea of creating DocumentOf was to allow a document that shares a similar structure (e.g. SVGDocument) be easily defined, but after taking a step back and looking deeper into the issue, including #3263, #3304 and DOM Bug 22960, I think my plan may be going too far and not the best approach.

A common reason why developers want to "cast" an Element to HTMLElement is to use the additional methods and properties provided by HTMLElement, but in my own experience, the cast is either not necessary or not enough. For example, when I need to disable a form control or set the style of a HTML element, the setAttribute method is usually used and no cast to HTMLElement is required. Event handlers may similarly be added to any Element using the addEventListener method. In fact, many UI events are not specific to HTMLElement according to the specification--they apply to all elements. I note that the commonly used click event is only defined in HTMLElement and missing in Element, but it appears to be an implementation issue of Edge that does not conform with the specification, which may be overridden. Returning HTMLElement instead of Element also does not help if I want to get the value of a form control-- a type assertion to a more specific type such as HTMLInputElement and HTMLSelectElement is in any event required.

One of the advantages of using TypeScript is to reduce, if not eliminate, the chance of calling an undefined method or property unexpectedly. Returning a type that may be wrong during runtime appears to undermine this. A developer should be aware of the actual type of element that he/she is working on. With the introduction of the as operator, I think we now have a way to assert the type of an element from the DOM elegantly.

Back to the present issue, I suggest changing parentElement to return Element without making parentElement in HTMLElement to return HTMLElement, as the latter case will be false when HTML is embedded in SVG. Type assertion should be added when the developer is sure that the parentElement is HTMLElement and wants to use the methods and properties that are not present in Element. That also makes the type definitions of TypeScript more consistent with and not significantly different from the DOM specification.

There may be additional issues to follow:

1. Should getElementById return Element instead of HTMLElement?
2. Should a new type, AttrOf<T extends Element>, be added to provide better support of ownerElement? _getAttributeNode and getAttributeNodeNS are rarely used. The cost of implementing AttrOf<T extends Element> is too high for 2 rarely used calls._
3. Should createElementNS(namespaceURI: "http://www.w3.org/1999/xhtml", qualifiedName: string): HTMLElement be added to Document and getElementsByTagNameNS(namespaceURI: "http://www.w3.org/1999/xhtml", localName: string): NodeListOf<HTMLElement> and getElementsByTagNameNS(namespaceURI: "http://www.w3.org/2000/svg", localName: string): NodeListOf<SVGElement> be added to both Document and Element? (#7712)
4. Should document.getElementsByName(elementName: string), which currently returns NodeListOf<Element>, be changed to return NodeListOf<HTMLElement>? It is defined in the HTML Specification to be applied to HTML elements only. (#7709)
5. Should more events, e,g. click, be moved from HTMLElement to Element with reference to the UI Events specification?

[mhegazy]

For 1, there has been a long discussion in https://github.com/Microsoft/TypeScript/issues/3613, the typings are not accurate, but looks like most ppl do not use SVG elements.

1. makes sense, can not think of a downside here.
2. sounds reasonable
3. I do not know :) what does the spec say?

[mindplay-dk]

Not sure if this is a separate or related issue, but (thanks to TS) I have gotten very comfortable writing vanilla DOM code, and wanted to try the same with SVG - and it doesn't seem like the type-definitions make much sense.

For starters:

var el = document.createElementNS("http://www.w3.org/2000/svg", "rect");

var parent = el.parentElement;

The type of parent here comes out HTMLElement. It seems like the parent element of any SVGElement except the root document SVGSVGElement should be expected to be another SVGElement? Likewise, most of the interface signatures seem to come out wrong, e.g. querySelector and querySelectorAll from within an SVG element, I'd expect, must return SVG rather than HTML elements?

Looks like the SVGElementInstance interface has all of the required type-definitions, but none of the actual SVG elements (such as SVGRectElement) inherit this type.

[ghost]

i ran into this when doing stuff with xml documents

This is arguable, because there was complainant before saying that it was too cumbersome to always cast the type Element to HTMLElement when in most common cases the actual type is HTMLElement, even though the spec says it should be Element.

For example, the return type of getElementById is defined as Element, however we made it HTMLElement to avoid too much casting. If the type is some other Element, you can just cast it to Element first and then cast it again to the final type. I think in most cases the parentElement is HTMLElement too, therefore it might be better just leave it the way it is.

this is the most arguable thing i've heard today

is breaking types for all non-html documents really the best fix for this

[karolinakuzniewicz]

What's the status of this issue in 2019? Right now the method is defined
as Document.getElementById(elementId: string): HTMLElement | null which cause an error if the element is SVGSVGElement and the workaround is to cast this element as SVGGraphicsElement | any type. But what is the final conclusion of this discussion? Should the method Document.getElementById(id) return Element or HTMLElement | SVGSVGElement | null? Because imo casting the types isn't anything good, is just a temporary fix.

[falsandtru]

Also see #32822.

[tmladek]

What's the status of this issue in 2020?

While I understand the rationale behind the decision to make the type HTMLElement instead of Element, considering it's plainly against the spec and causing trouble for people working with non-HTML documents, I would urge to reconsider.

Or as a compromise, given the overwhelming majority use-case, I'd be in favor of making it a tsconfig.json option as suggested in #32822, I believe this has potential to satisfy both use cases.

[RyanCavanaugh]

What's the status of this issue in 2020?

[orta]

@jun-sheaf sent a PR for these changes and I've been reviewing about whether it should make it in for 4.1.

I don't think we should make these changes, because it's going to add break a lot of code in a way that people won't think is to their advantage. TS tries to balance correctness and productivity and I think making getElementById start returning code which needs to be casted to get the same tooling support we have today in the majority of cases (e.g. HTML nodes in a JS context) is going to be a bad call for TypeScript.

The main crux of the PR is changes like these:

-  getElementById(elementId: string): HTMLElement | null;
+  getElementById<E extends Element = Element>(elementId: string): E | null;

Meaning in the old version to use an SVG/XML element meant doing something like:

const logo = document.getElementById("my-logo") as unknown as SVGPathElement

Which isn't great, and now you can write:

const logo2 = getElementById2("my-logo") as SVGPathElement
const logo3 = getElementById2<SVGPathElement>("my-logo")

Which is better.

But we lose something in the trade-off, and that is the tooling support for JS (because it always uses the defaults)

With the changes to make it exactly the same as the spec, you can no-longer write code like:

const logo4 = getElementById2("my-logo")!
logo4.innerText = "123"

Without a cast, which would hit the TypeScript Website's codebase a few times (but _I could_ switch to . textContent in those cases) and you lose the addEventListener/removeEventListener auto-complete map: e.g.

logo2.addEventListener("|
> "fullscreenchange" | "fullscreenerror"

logo2.addEventListener("|
> "fullscreenchange" | "fullscreenerror" | "abort" | "animationcancel" | "animationend" | "animationiteration" | "animationstart" | "auxclick" | "blur" | "cancel" | "canplay" | ... 80 more ... | "paste"

And this means you don't get a typed event anymore. Which I don't think is worth the change to getElementById<E extends Element = Element>(elementId: string): E | null.

Switching it to getElementById<E extends Element = HTMLElement>(elementId: string): E | null on the other hand still allows for setting the return type less drastic ways for the uncommon cases:

const logo2 = getElementById2("my-logo") as SVGPathElement
const logo3 = getElementById2<SVGPathElement>("my-logo")

But doesn't hinder the default JS tooling support.

Notes

[tmladek]

Hate to interject, as I do agree with your assessment of the trade-offs involved, but I would like to hear your opinion on making it a project-wide setting (#32822) - wouldn't that also alleviate the productivity/JS default issues?

[orta]

Personally, I'm super hesitant to add more flags and options to TS, there's already 100+ - but I'd let the TS team as a whole make that call

IMO, the strictest/most compliant version can ship as a .d.ts which can modify the DOM types to be stricter globally though:

// Default with lib.dom.d.ts
interface Document {
    getElementById<E extends Element = HTMLElement>(elementId: string): E | null
}

// In something like in  @types/strict-dom/index.d.ts
interface Document {
    getElementById<E extends Element = Element>(elementId: string): E | null
}

const a = document.getElementById("e")!
a.innerText // bails

Which means this can effectively live in user-land instead of it having to be inside the compiler, having a library means it can support more than just the recommendations here too