Skip to content

document.createElement returns an HTMLElement#7136

Closed
trusktr wants to merge 1 commit into
mdn:mainfrom
trusktr:patch-3
Closed

document.createElement returns an HTMLElement#7136
trusktr wants to merge 1 commit into
mdn:mainfrom
trusktr:patch-3

Conversation

@trusktr

@trusktr trusktr commented Jul 21, 2021

Copy link
Copy Markdown
Contributor

What was wrong/why is this fix needed? (quick summary only)

document.createElement more accurately returns an HTMLElement instances. document.createElementNS is needed (unfortunately) for non-HTMLElement types, like SVGElement.

The built-in type definition in TypeScript also returns HTMLElement.

Issue number (if there is an associated issue)

Anything else that could help us review it

@trusktr
trusktr requested review from a team and jpmedley and removed request for a team July 21, 2021 15:59
@github-actions

Copy link
Copy Markdown
Contributor

Preview URLs

Flaws

URL: /en-US/docs/Web/API/Document/createElement
Title: Document.createElement()
on GitHub
Flaw count: 2

  • macros:
    • /en-us/docs/web/api/parentnode (url: /en-US/docs/Web/API/ParentNode) does not exist
    • /en-us/docs/web/api/geometryutils (url: /en-US/docs/Web/API/GeometryUtils) does not exist

External URLs

URL: /en-US/docs/Web/API/Document/createElement
Title: Document.createElement()
on GitHub

No new external URLs

@jpmedley

Copy link
Copy Markdown
Collaborator

@Rumyra,

Chrome still returns Element. (It's probably out of date with the spec.) I'm not sure how to handle this, but at the very least, we'll need to add notes to the BCD indicating when various browsers changed.

@Rumyra

Rumyra commented Jul 22, 2021

Copy link
Copy Markdown
Collaborator

Thanks @jpmedley - a quick sanity check and FF returns HTMLElement. The question is, how do we find out when browsers updated this? Maybe we add a note to BCD explaining that Chrome returns Element - then we can update that once Chrome has updated

@jpmedley

jpmedley commented Jul 22, 2021

Copy link
Copy Markdown
Collaborator

@Rumyra Here's the PR for that.

@foolip

foolip commented Jul 23, 2021

Copy link
Copy Markdown
Contributor

document.createElement() doesn't always return an HTMLElement instance. The spec for this is here:
https://dom.spec.whatwg.org/#dom-document-createelement

The return type in Web IDL is Element, and what is returned depends on the document's type.

Here's a snipped where the returned Element wouldn't be a HTMLElement:

var doc = document.implementation.createDocument(null, null, null); // or new Document()
var el = doc.createElement('bla');
// Object.getPrototypeOf(el) is Element.prototype

I believe this is ancient behavior.

<h3 id="Return_value">Return value</h3>

<p>The new {{domxref("Element")}}.</p>
<p>The new {{domxref("HTMLElement")}}.</p>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though the return type is accurately described as Element, it would be useful to document when an HTMLElement is returned, and that this is the case that web developers will see 99.9% of the time.

@jpmedley

Copy link
Copy Markdown
Collaborator

@foolip Good catch. This is something we should document completely.

@trusktr

trusktr commented Aug 4, 2021

Copy link
Copy Markdown
Contributor Author

@foolip Interesting. So what the spec says is actually not what practically all browser developers experience. They don't deal with different types of "documents", only window.document documents from with Windows's document properties, which are all one type of Document.

It seems the spec covers cases for other engines that aren't Firefox, Chrome, Safari, or Edge (which ones?). This is so much the case, that even the official TypeScript type definition returns HTMLElement:

https://github.com/microsoft/TypeScript/blob/03dff41c9f2038f66fb358e5c23ebd7271145978/lib/lib.dom.d.ts#L4464

To follow the spec? Or to follow what we all see in practice? That is the question. (It seems the latter is more useful).

@jpmedley

jpmedley commented Aug 5, 2021

Copy link
Copy Markdown
Collaborator

This is a reference for web developers explaining what the web does, not a reference for browser implementors explaining what it should do. That's what specs are for.

@jpmedley

jpmedley commented Aug 5, 2021

Copy link
Copy Markdown
Collaborator

It seems the spec covers cases for other engines that aren't Firefox, Chrome, Safari, or Edge (which ones?).

I'm not sure what would lead you to this conclusion. It's more likely that browser vendors haven't gotten around to updating their implementations for one reason or another. It's also possible that someone was supposed to update the spec and didn't. I would suggest opening an issue on the spec repo itself. The editors of the spec should be able to clear this up.

@foolip

foolip commented Aug 5, 2021

Copy link
Copy Markdown
Contributor

Maybe there's more than meets the eye here, but AFAICT there isn't anything funny going on, I believe browsers match the spec.

Since HTMLElement inherits from Element it would be correct (but less precise than possible) to say that a method that always returns a HTMLElement object has a return type of Element. In the case of document.createElement the returned object is typically a HTMLElement but it can also be a plain Element.

It seems to me the TypeScript bindings are wrong here. Maybe the return type could be made to depend on the type of this?

@Ryuno-Ki

Ryuno-Ki commented Aug 7, 2021

Copy link
Copy Markdown
Collaborator

Pinging @sandersn since he reviewed changes on this part of the TypeScript code base

@Rumyra

Rumyra commented Aug 8, 2021

Copy link
Copy Markdown
Collaborator

I would like to - as soon as I get 5 mins - close this PR in favour for the explanation @foolip has given to be included on this page.

@sandersn

sandersn commented Aug 11, 2021

Copy link
Copy Markdown

@orta has thought about this particular problem more than I have, but to quote two early comments:

@foolip

it would be useful to document when an HTMLElement is returned, and that this is the case that web developers will see 99.9% of the time.

@trusktr

To follow the spec? Or to follow what we all see in practice? That is the question. (It seems the latter is more useful).

Typescript aims for the 99.9% case here, not spec compatibility. That's why it intentionally returns HTMLElement even though it won't be correct sometimes.

Edit: For Typescript, the penalty for returning a subclass is low: the people who find themselves in the 0.01% case will see extraneous HTMLElement properties in completions when they should see only the Element ones.

@jpmedley

Copy link
Copy Markdown
Collaborator

Web developers don't come here to learn what's supposed to happen. They come here to learn what actually happens.

@foolip

foolip commented Aug 11, 2021

Copy link
Copy Markdown
Contributor

@jpmedley this isn't a spec-fiction vs. browser-reality situation, it's an issue of HTMLElement usually being returned except when dealing with non-HTML documents.

With the code snippet in #7136 (comment), what actually happens is that Chrome, Firefox and Safari return an Element instance. This is also what's supposed to happen per spec.

MDN could say that an HTMLElement is returned if the document is an HTMLDocument, which is most common case, and otherwise an Element is returned.`

@orta

orta commented Aug 12, 2021

Copy link
Copy Markdown

Yeah, just confirming @sandersn - we chose HTMLElement for a few API cases like this which are specifically not conforming to the spec but conform to what happens nearly all the time. We have tried to switch down to Element but use generics to default to HTMLElement ( microsoft/TypeScript-DOM-lib-generator#885 ) but found that the results would really make JS folks lives worse microsoft/TypeScript#4689 (comment)

IMO #7136 (comment) is probably the right take 👍🏻

@Ryuno-Ki

Copy link
Copy Markdown
Collaborator

Thanks for chiming in and providing the rationale behind the decisions made in TypeScript 🚀

@Rumyra

Rumyra commented Aug 16, 2021

Copy link
Copy Markdown
Collaborator

Closing this PR in favour of this one #7971 which adds a note based on this discussion 👍

@Rumyra Rumyra closed this Aug 16, 2021
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 16, 2022
@trusktr
trusktr deleted the patch-3 branch January 5, 2024 08:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants