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

NodeList, NodeListOf<Element> and HTMLCollection #424

Closed
FranklinWhale opened this issue Aug 11, 2014 · 9 comments
Closed

NodeList, NodeListOf<Element> and HTMLCollection #424

FranklinWhale opened this issue Aug 11, 2014 · 9 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

@FranklinWhale
Copy link

Currently, the following DOM methods has a return type definition of NodeList, but the type of nodes returned is actually always Element:

getElementsByClassName
getElementsByTagNameNS
getElementsByTagName(name: string)
getElementsByName
msElementsFromPoint
msElementsFromRect

As a result, a cast to Element is required before any method of Element can be used. To remove the need of the cast, I suggest changing their return type definition to NodeListOf<Element>.

Also, there are some HTML DOM properties (e.g. document.forms) having a return type definition of HTMLCollection, but HTMLCollection does not extend NodeListOf<HTMLElement>. That leads to some difficulties in creating a method that takes a list of elements. I suggest changing HTMLCollection to extend NodeListOf<HTMLElement>.

Finally, should a List<T> interface with the following definition be created, such that Array<T>, NodeList, NodeListOf<HTMLElement> and HTMLCollection all extend to List<T>?

interface List {
    length: number;
    [index: number]: T;
}
@RyanCavanaugh
Copy link
Member

Good feedback. We'll need to look at this again when we rewrite the lib.d.ts generator.

@taylorhutchison
Copy link

Still being burned by this issue. getElementsByTagName (and other DOM Element selectors) seem to have a long-running history (documented here: https://bugzilla.mozilla.org/show_bug.cgi?id=14869) of returning HTMLCollection instead of NodeList.

@mhegazy mhegazy added the Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript label Mar 24, 2015
@duanyao
Copy link

duanyao commented May 18, 2015

In many cases, we want certain sub-interface of Element to be returned from those DOM APIs.

Firstly, I suggest making those API as generic methods, e.g.:

interface NodeSelector {
  querySelector<E extends Element>(sel : string): E;
  querySelectorAll<E extends Element>(sel : string): NodeListOf<E>;
  //...
}

Additionally, I suggest making the non-generic versions return a UniversalElement or a list of it. UniversalElement is an imaginary Element that extends all known sub-interfaces of Element. Say:

interface UniversalElement extends
  HTMLAElement,
  HTMLInputElement,
  //...
  SVGLineElement,
  SVGRectElement,
  //...
 {}

interface NodeSelector {
  //...
  querySelector(sel : string): UniversalElement;
  querySelectorAll(sel : string): NodeListOf<UniversalElement>;
}

Rationales:

  • UniversalElement makes most type assertions unnecessary in DOM related codes, so that greatly simplifies those codes.
  • APIs like querySelector can actually return any sub type of Element, depending on the parameter and context, so it is not that crazy to let them return UniversalElement by default.
  • This change won't break existing (correct) codes.
  • Maybe UniversalElement makes it easier for developers to produce type errors in rare cases (I haven't figured out one however), but they can use the generic version of those APIs to narrow down the return type if necessary.

However, there is a member conflicting problem: SVGElement has a few conflicting members with HTMLElement. For example className in HTMLElement is a string, but in SVGElement is a SVGAnimatedString.

One possible solution is also making className unified:

interface UniversalClassName extends String, SVGAnimatedString {}

interface UniversalElement {
  className: UniversalClassName;
}

However, because primitive types in TS are not interfaces, and an instance of interface String can't be used as a string directly, so you can't pass UniversalClassName to a context where string is expected; a cast or conversion is needed. Still wondering how to workaround this limitation. Is this a TS bug?This seems can't be easily workarounded (see http://typescript.codeplex.com/discussions/459084). So this solution has too high a price to pay.

Another solution is kicking out SVG's funny className and other incompatible properties from UniversalElement. After all, we use HTMLElement most of the time. We may create a PartialUniversalSVGElement interface which gathers all SVG elements' members except incompatible ones, and let UniversalElement extend it.

@mhegazy mhegazy added Help Wanted You can do this and removed Revisit An issue worth coming back to labels Dec 10, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Dec 10, 2015

PRs welcomed, here is some information about submitting lib.d.ts changes: https://github.com/Microsoft/TypeScript/blob/master/CONTRIBUTING.md#contributing-libdts-fixes

@crash-dive
Copy link

I am happy to add this. But I need to know what would be accepted. I was thinking of adding a new type modelled on NodeListOf called HTMLCollectionOf and then adding generic methods for the collection methods. For example I have these in the project I have open at the moment:

interface Document
{
    getElementById<THTMLElement extends HTMLElement>(elementId: string): THTMLElement;
    getElementsByClassName<THTMLElement extends HTMLElement>(classNames: string): NodeListOf<THTMLElement>;
}

interface Element
{
    getElementsByClassName<THTMLElement extends HTMLElement>(classNames: string): NodeListOf<THTMLElement>;
}

interface NodeSelector {
    querySelector<THTMLElement extends HTMLElement>(selectors: string): THTMLElement;
    querySelectorAll<THTMLElement extends HTMLElement>(selectors: string): NodeListOf<THTMLElement>;
}

Not an exhaustive list but just an example I have to hand.

Thoughts?

@mhegazy
Copy link
Contributor

mhegazy commented Jan 22, 2016

@crash-dive this looks like can be handled in a separate PR. the OP was about 1. adding a new HTMLCollectionOf<T extends HTMLElement> extends HTMLCollection and wiring this in the correct placeds. 2. switch msElementsFromRect and msElementsFromPoint to return NodeListOf<Element>, and possibly others that match their style. and 3. make NodeList, NodeListOf, HTMLElementCollection, HTMLElementCollectionOf to extend from ArrayLike

@mhegazy mhegazy added this to the Community milestone Jan 22, 2016
@FranklinWhale
Copy link
Author

A PR has been submitted for adding HTMLCollectionOf. There will be another PR for msElementsFromRect and msElementsFromPoint later. For #3, I think an explicit extension of ArrayLike is not necessary because of structural typing.

@FranklinWhale
Copy link
Author

A second PR for this issue has been made. I note that form.elements should be changed to return HTMLFormControlsCollection, but that change, in my view, should be tracked by another issue. I think this issue may be closed after the second PR is accepted.

@kumaresan-subramani
Copy link

kumaresan-subramani commented Apr 4, 2018

onreset(element: MouseEvent) : void { let parentNode: NodeListOf<HTMLElement> = document.getElementsByClassName('box-form')[0].querySelectorAll('.e-float-text'); for (let i: number = 0; i < parentNode.length; i++) { parentNode[i].classList.remove('e-label-top'); parentNode[i].classList.add('e-label-bottom'); } }

this is my code, in this case i met following error

Type 'NodeListOf' is not assignable to type 'NodeListOf'.
Type 'Element' is not assignable to type 'HTMLElement'.
Property 'accessKey' is missing in type 'Element'.

@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

No branches or pull requests

8 participants