Skip to content

querySelector and querySelectorAll should support generics for "string" selectors #12568

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
tomwanzek opened this issue Nov 30, 2016 · 8 comments · Fixed by microsoft/TypeScript-DOM-lib-generator#170
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

@tomwanzek
Copy link

tomwanzek commented Nov 30, 2016

TypeScript Version: 2.2.0-dev.20161129

Code

As of the TS version stated above the NodeSelector in lib.es6.d.ts interface is defined as:

interface NodeSelector {
    querySelector<K extends keyof ElementTagNameMap>(selectors: K): ElementTagNameMap[K] | null;
    querySelector(selectors: string): Element | null;
    querySelectorAll<K extends keyof ElementListTagNameMap>(selectors: K): ElementListTagNameMap[K];
    querySelectorAll(selectors: string): NodeListOf<Element>;
}

Expected behavior:

It should be possible to use a generic to control the return type of the general string-based selection methods, rather than being returned a fixed return type of Element. I.e. assume one uses a class my-svg-shape which one exclusively applies to either an SVG rect or circle.

There should be method signatures to control the return type at the developer's discretion, i.e.

querySelector<E extends Element>(selectors: string): E | null;
querySelectorAll<E extends Element>(selectors: string): NodeListOf<E>;

These would obviously fall back to the current behavior, if used without specifying a type, but could be used as, e.g.

let x = document.querySelectorAll<SVGRectElement | SVGCircleElement>('.my-svg-shape');

Actual behavior:

Return type is fixed to Element

EDIT: Fixed copy-paste typo in application example. Inserted .my-svg-shape as selector string.

@aluanhaddad
Copy link
Contributor

Actually I think this is supposed to work. The purpose of the interface is to enable it to return the correct types as mapped. The problem is that key is not being inferred as a literal.

If you use this definition it works

interface NodeSelector {
    querySelector<K extends string & keyof ElementTagNameMap>(selectors: K): ElementTagNameMap[K] | null;
    querySelector(selectors: string): Element | null;
    querySelectorAll<K extends string & keyof ElementListTagNameMap>(selectors: K): ElementListTagNameMap[K];
    querySelectorAll(selectors: string): NodeListOf<Element>;
}

@tomwanzek
Copy link
Author

@aluanhaddad the use case I described, i.e. union types for (possibly) mixed type selection, is not covered by the latest lib.es6.d.ts definitions as far as I see. That's what this issue is aiming to get addressed.

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Dec 2, 2016

Sorry I missread the original comment. At the time these declarations were not returning the specific types even for non-unions

@mhegazy mhegazy added Help Wanted You can do this Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Suggestion An idea for TypeScript labels Dec 2, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Dec 2, 2016

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.

@tomwanzek
Copy link
Author

@YuichiNukiyama thanks for tackling this so quickly!

@mhegazy thanks for the link, seems I am too late this time around...

@FranklinWhale
Copy link

What is the difference between using generic and type assertion?

let x = document.querySelectorAll('.my-svg-shape') as NodeListOf<SVGRectElement | SVGCircleElement>; seems fine.

@tomwanzek
Copy link
Author

@FranklinWhale IMHO as is a last resort when a more targeted approach is not available. It feels a lot more natural to use the generic with extends Element as the base line, the interface already uses generics, so it also seems idiomatically preferable.

@tomwanzek
Copy link
Author

Is there any update on whether the PR will be accepted?

@mhegazy mhegazy self-assigned this Aug 15, 2017
@mhegazy mhegazy added the Fixed in TSJS repo Fix merged in https://github.com/Microsoft/TSJS-lib-generator, but not ported yet label Aug 15, 2017
@mhegazy mhegazy added this to the TypeScript 2.6 milestone Aug 15, 2017
@mhegazy mhegazy reopened this Aug 15, 2017
@mhegazy mhegazy added Fixed A PR has been merged for this issue and removed Fixed A PR has been merged for this issue Fixed in TSJS repo Fix merged in https://github.com/Microsoft/TSJS-lib-generator, but not ported yet labels Aug 24, 2017
@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
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