Skip to content

Element.closest should return specific types for tag selectors #17550

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
thw0rted opened this issue Aug 1, 2017 · 7 comments · Fixed by #20177
Closed

Element.closest should return specific types for tag selectors #17550

thw0rted opened this issue Aug 1, 2017 · 7 comments · Fixed by #20177
Assignees
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

@thw0rted
Copy link

thw0rted commented Aug 1, 2017

TypeScript Version: 2.4.0

Code

let a = document.querySelectorAll("a");
let div = a.closest("div");

Expected behavior:

a is an HTMLAnchorElement, and div is an HTMLDivElement.

Actual behavior:

el2 is actually an Element because Element.closest always returns an Element. This was fixed for querySelector in #8114 and the same behavior should be applied to closest.

@thw0rted thw0rted changed the title Element.closest should return Element.closest should return specific types for tag selectors Aug 1, 2017
@thw0rted
Copy link
Author

thw0rted commented Aug 1, 2017

Would you look at that, a hack-job cut and paste worked on the first try! I'm running a vanilla-JS project in VSCode but when I put this in my globals.d.ts, it fixed the issue:

declare global {
    interface Element extends Node, GlobalEventHandlers, ElementTraversal, NodeSelector, ChildNode, ParentNode {
        closest<K extends keyof ElementTagNameMap>(selector: K): ElementTagNameMap[K] | null;
    }
}

I copied that verbatim from NodeSelector.querySelector in lib.dom.d.ts. Any reason I shouldn't make a PR for this? (I don't really speak TS yet so I'm not sure if I'm doing something dumb here.)

@thw0rted
Copy link
Author

thw0rted commented Aug 1, 2017

Out of curiosity I searched lib.dom.d.ts for "selector" and the only three places where a method takes a "selector" argument are Element.closest, NodeSelector.querySelector, and NodeSelector.querySelectorAll -- the last 2 already implement "intelligent return types", so this improvement to closest would close the gap.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 21, 2017

quertySelectorAll returns a NodeList of HTMLAnchorElement, and not an HTMLAnchorElement

image

@mhegazy mhegazy closed this as completed Aug 21, 2017
@mhegazy mhegazy added the Working as Intended The behavior described is the intended behavior; this is not a bug label Aug 21, 2017
@jcready
Copy link

jcready commented Aug 21, 2017

@mhegazy I believe that was just a typo. Please re-open this issue.

Code:

let a = document.querySelector("a");
if (!a) throw new Error();
let div = a.closest("div");

Expected behavior:

div is HTMLDivElement | null.

Actual behavior:

div is actually Element | null because Element.closest always returns Element | null. This was fixed for querySelector in #8114 and the same behavior should be applied to closest.

@mhegazy mhegazy reopened this Aug 21, 2017
@mhegazy mhegazy added Suggestion An idea for TypeScript Help Wanted You can do this Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels Aug 21, 2017
@mhegazy mhegazy added this to the Community milestone Aug 21, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Aug 21, 2017

PRs welcomed. You can find more information about contributing lib.d.ts fixes at https://github.com/Microsoft/TypeScript/blob/master/CONTRIBUTING.md#contributing-libdts-fixes.

@thw0rted
Copy link
Author

Yes, sorry, typo -- I had meant querySelector not querySelectorAll in my example code. Obviously both should return the most specific type possible.

@jthoms1
Copy link

jthoms1 commented Nov 2, 2017

Seems like it is missing the following line. I can submit a PR if this seems correct. @mhegazy

closest<K extends keyof ElementTagNameMap>(selector: K): ElementTagNameMap[K] | null;

@mhegazy mhegazy self-assigned this Nov 3, 2017
@mhegazy mhegazy modified the milestones: Community, TypeScript 2.7 Nov 3, 2017
@mhegazy mhegazy added the Fixed in TSJS repo Fix merged in https://github.com/Microsoft/TSJS-lib-generator, but not ported yet label Nov 3, 2017
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Nov 21, 2017
@mhegazy mhegazy removed the Fixed in TSJS repo Fix merged in https://github.com/Microsoft/TSJS-lib-generator, but not ported yet label Nov 21, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 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.

4 participants