Skip to content

XML getElementsByTagName returns Node #3613

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
NekR opened this issue Jun 24, 2015 · 15 comments
Closed

XML getElementsByTagName returns Node #3613

NekR opened this issue Jun 24, 2015 · 15 comments
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue

Comments

@NekR
Copy link

NekR commented Jun 24, 2015

Hi,

I understand all concerns around HTML/SVG elements with querySelector(), etc. But why method which should return Element cannot even return Element but returns Node? Do you think it's possible to query processing instructing with getElementsByTagName()? I know it's NodeList returned by that method, but NodeList does not mean that it always should contains only Node class, not its subclasses.

This code won't work:

let xml: XMLDocument;
let element = xml.getElementsByTagName('test')[0];

element.getAttribute('attr');

Also, fine thing here is that Node has method hasAttributes().

@kitsonk
Copy link
Contributor

kitsonk commented Jun 24, 2015

As per the W3C DOM Level 3, NodeList is an array-like structure containing items of Node. And Node does not contain getAttribute.

The easiest way to deal with this, because you know something that can't be derived from the code, is to use a cast:

let xml: XMLDocument;
let element = <Element> xml.getElementsByTagName('test')[0];

element.getAttribute('attr'); /* no error */

@mhegazy mhegazy added the Question An issue which isn't directly actionable in code label Jun 24, 2015
@NekR
Copy link
Author

NekR commented Jun 24, 2015

@kitsonk Oh, man, I know how it's per spec, but I am a bit tired of such answers here. It's not a question how to solve this problem, I understand how to cast types. Problem is that getElementsByTagName() cannot return anything than Element, am I wrong?
And if you are so spec funs, why then getElementsByTagName('a') in HTML document returns NodeListOf<HTMLAnchorElement>?

Plus do not miss this:

Also, fine thing here is that Node has method hasAttributes().

@NekR
Copy link
Author

NekR commented Jun 24, 2015

As per the W3C DOM Level 3, NodeList is an array-like structure containing items of Node. And Node does not contain getAttribute.

And please, as per spec, Element inherits from Node, right? I believe it's true because this code returns true in browsers:

document.getElementsByTagName('a')[0] instanceof Node &&
document.getElementsByTagName('a')[0] instanceof Element

So returning Element won't be any incorrect per-spec, if you always like to be per-spec (but you are not of course in some other cases, but who cares).

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript and removed Question An issue which isn't directly actionable in code labels Jun 24, 2015
@kitsonk
Copy link
Contributor

kitsonk commented Jun 25, 2015

Oh, man, I know how it's per spec, but I am a bit tired of such answers here.

If you think I am affiliated with TypeScript, you are wrong. I am trying to point out though that your expectations do not align to the specs, so I am surprised you think that TypeScript shouldn't follow the specs and do what you think it should do, versus what the spec says. I even gave you a working piece of code that gets around the issue. So to be honest, I am "tired of such" attitude for just trying to help.

Why do you think TypeScript should magically ignore the spec for you?

Yes, you are right that Element inherits from Node. But Node !== Element, just because at runtime, in 99% percent of the cases, it only returns something an Element doesn't mean that it is always the case. In fact, you are using the one thing a lot of other developers argue isn't "realistic" in that you are using an XMLDocument, when others argue Document is just fine.

There are several other issues that talk about the usability of the lib.d.ts in practice and the TypeScript team have indicated they are thinking about the right way to deal with it. I am sure if you have bright ideas of how to make it work, they would be glad to hear from you.

As far as hasAttributes() and why Node has it, but not getAttribute, you should be mad at the W3C, because that is as per the spec. There is a lot in the DOM that I consider awkward and challenging. Maybe if you e-mail them, they will change the spec. Let me know how you get on with that.

@duanyao
Copy link

duanyao commented Jun 25, 2015

Note that the DOM spec is written in a language called WebIDL. The mapping of WebIDL to ECMAScript is defined in WebIDL spec, but is undefined for other languages. So it is up to TS to decide how to map from DOM spec. Also note that some WebIDL features are not possible in TS currently, such as overriding a member in a sub interface.

As to getElementsByTagName(), the DOM spec says "Returns a NodeList of all the Elements ...", so it is perfectly safe and logical to return a NodeListOf<Element> in TS.

@kitsonk
Copy link
Contributor

kitsonk commented Jun 25, 2015

@duanyao that is a fair point about getElementsByTagName(). Thanks for taking the time to look up information and reference it.

As far as the WebIDL, if I understand correctly, lib.d.ts is auto-generated from the internal implementation of IE's ECMAScript engine, which is based on Chakra's implementation of the DOM spec. Ergo, I suspect Chakra is returning a NodeListOf<Node>. Obviously @RyanCavanaugh felt this was a bug so hopefully the team will be willing to vary the lib.d.ts for usability.

@duanyao
Copy link

duanyao commented Jun 25, 2015

@kitsonk Note that WebIDL has no generics. AFIW NodeListOf<N> and related things are manual tweaks on top of auto-generated lib.d.ts, and lib.d.ts is auto-generated from WebIDL files of IE 11 (those files are also used to generated C++ bindings of DOM objects). JS engines only understand JS, while DOM and other web specs are implemented in C++.

@NekR
Copy link
Author

NekR commented Jun 25, 2015

@kitsonk You know, after your answers here I really do not want to report any bugs here to TypeScript. If you are tired here, then please, do not answer here, it's simple. You was wrong here but was arguing with such rage like only you can be right.

It's so obvious that getElementsByTagName returns Elements. Treating the spec so literally it's like junior Police man (or any such position, like moderator somewhere) treats rules literally word-to-word. It's of course wrong, their job is to be people, so please, be people here.

@NekR NekR closed this as completed Jun 25, 2015
@RyanCavanaugh
Copy link
Member

If you don't mind, I'd like to leave this open so that we can fix the bug?

@RyanCavanaugh RyanCavanaugh reopened this Jun 25, 2015
@mhegazy mhegazy added this to the TypeScript 1.6 milestone Jun 25, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Jun 25, 2015

@zhengbli can you take a look.

@zhengbli
Copy link
Contributor

I agree that it is safe to assume that getElementsByTagName() returns only NodeList<Element>. And similar functions include getElementsByClassName, getElementsByName and getElementsByTagNameNS.

@mhegazy
Copy link
Contributor

mhegazy commented Jun 26, 2015

and NodeSelector.querySelectorAll as well.

@delebash
Copy link

I agree this should not be throwing an error. Example code
No error thrown because it is ById which is HtmlElement and defined correctly in lib.d.ts

document.getElementById('left-aside-wrapper')[0].classList.toggle('isOpen'); 

Error thrown Property 'classList' does not exist on type 'Node'.

document.getElementsByClassName('left-aside-wrapper'[0].classList.toggle('isOpen');

I understand this is because it is not defined in the lib.d.ts file but this is the is related to the issue from OP.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 13, 2015

@NekR give the latest library a test and let us know if the issue has been solved.

@NekR
Copy link
Author

NekR commented Jul 13, 2015

@mhegazy it seems to work fine. Thanks!

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

7 participants