Skip to content
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

Node.childNodes return type appears to be slightly inaccurate #18194

Closed
dsifford opened this issue Aug 31, 2017 · 2 comments
Closed

Node.childNodes return type appears to be slightly inaccurate #18194

dsifford opened this issue Aug 31, 2017 · 2 comments
Labels
Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue Help Wanted You can do this Suggestion An idea for TypeScript

Comments

@dsifford
Copy link
Contributor

Greetings!

Ran into this earlier today and I'm wondering if it's a bug or if I'm just mistaken for what I assumed would be returned.

Below is a snapshot of the definition of Node.

https://github.com/Microsoft/TypeScript/blob/c7b4ed3a91964915b953b34ad2ae72f36e7d6efe/lib/lib.dom.d.ts#L8260-L8265

Currently, Node.childNodes returns a NodeList which is mostly correct. However, wouldn't it be more accurate to instead return a NodeListOf<Node & ChildNode> so that the remove method is exposed?

Example of the issue:

declare const node: Node;

for (const child of node.childNodes) {
    child.remove(); // fails "Property 'remove' does not exist on type 'Node'
}

Happy to submit a PR for this if needed (I'm going to also submit one for the issue I opened yesterday when I get some spare time this weekend).

Thanks in advance 😄

@mhegazy
Copy link
Contributor

mhegazy commented Aug 31, 2017

@kitsonk thoughts?

@mhegazy mhegazy added the Needs Investigation This issue needs a team member to investigate its status. label Aug 31, 2017
@kitsonk
Copy link
Contributor

kitsonk commented Sep 1, 2017

The living standard says that:

The childNodes attribute’s getter must return a NodeList rooted at the context object matching only children.

and that a child is:

An object that participates in a tree has a parent, which is either null or an object, and has children, which is an ordered set of objects. An object A whose parent is object B is a child of B.

But of course there is a challenge in modelling that in Web IDL, because the types in there don't have the concept of generics, so both the IDL from Edge and the WHATWG IDL don't say anything but NodeList. So it seems totally appropriate for it to be NodeListOf<Node & ChildNode> as that is clearly more spec aligned.

@mhegazy mhegazy added Suggestion An idea for TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Help Wanted You can do this and removed Needs Investigation This issue needs a team member to investigate its status. labels Sep 5, 2017
@mhegazy mhegazy added this to the Community milestone Sep 5, 2017
@mhegazy mhegazy modified the milestones: Community, TypeScript 2.9, TypeScript 2.8 Mar 9, 2018
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Mar 9, 2018
@microsoft microsoft locked and limited conversation to collaborators Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants