Node.parentElement should be Element, not HTMLElement

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).

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.

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>>>

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.

Also see #32822.

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.

What's the status of this issue in 2020?

@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

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?

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

Revert "Replaces several DOM methods with weaker typings and closes m…

Related question on Stack Overflow: Why isn't the style property suggested for the result of document.querySelector()?

Merge pull request microsoft#885 from jun-sheaf/master

Navigation Menu

Repository navigation

Node.parentElement should be Element, not HTMLElement #4689

14 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Node.parentElement should be Element, not HTMLElement #4689

Description

Activity

zhengbli commented on Oct 7, 2015

leonyu commented on Nov 22, 2015

14 remaining items

karolkarolka commented on Jan 21, 2019

falsandtru commented on Aug 12, 2019

tmladek commented on Jul 6, 2020

RyanCavanaugh commented on Jul 8, 2020

orta commented on Sep 10, 2020

tmladek commented on Sep 10, 2020

orta commented on Sep 10, 2020

starball5 commented on Mar 5, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions