Changed return type of toArray to HTMLElement[]

[LOZORD]

case 2. Improvement to existing type definition.

• documentation or source code reference which provides context for the suggested changes. url http://api.jquery.com/html .
  • it has been reviewed by a DefinitelyTyped member.

NOTE Also changed the return type for get(void) and added a JSDoc @alias annotation.

Documentation See toArray() and get(void).

Due to legacy support, these functions are practically aliases.

[dt-bot]

jquery/jquery.d.ts

to authors (@borisyankov @choffmeister @Steve-Fenton @Diullei @tasoili @jasons-novaleaf @seanski @Guuz @ksummerlin @basarat @nwolverson @derekcicerone @AndrewGaspar @jameshfisher @seikichi @benjaminjackman @s093294 @JoshStrobl @johnnyreilly @DickvdBrink). Could you review this PR?
👍 or 👎?

Checklist

• pass the Travis CI test?

[LOZORD]

Failure: https://travis-ci.org/DefinitelyTyped/DefinitelyTyped/builds/158300393#L1213

Should I be using HTMLElement? It seems that the vast majority of the definitions use Element instead of HTMLElement.

[thorn0]

Think of $(window).toArray()

[LOZORD]

@thorn0, that's a fair point, but I feel like that is kind of a "garbage-in-garbage-out scenario". When would you need to actually use $(window).toArray()? Maybe definitions need to be made more strict?

[johnnyreilly]

Could you address the failing tests please? If Element is what's generally used now I'd stick with that for consistency. If there's a need then moving from Element to HTMLElement could be handled in a subsequent PR which addresses all instances at once. But I'm not convinced it's necessary really.

[sheetalkamat]

Thank you for your contribution.
Can you please fix the failing Travis CI tests and update the PR!

[LOZORD]

Ok, tests pass @johnnyreilly @sheetalkamat 🎉

I switched Element for HTMLElement to get the tests to pass. It's up to you whether you want the "generality" (and majority) of Element or not.

[johnnyreilly]

Thanks!

[dedocibula]

Hi, having a problem with this when using in conjunction with mapping function.

For instance:
$('a').map((_, a) => a.href).get()
or
$('a').map((_, a) => a.href).toArray()

Expected output should be array of strings but Typescript compiler won't allow cast from HTMLElement[] to string[]. any[] used to work fine with additional casting. Please, reconsider.

[LOZORD]

Why not first call `toArray` and map off of that? It should be equivalent, with the benefit of still being type safe.

…

On Mon, Feb 27, 2017 at 2:55 PM Andrej Galad ***@***.***> wrote: Hi, having a problem with this when using in conjunction with mapping function. For instance: $('a').map((_, a) => a.href).get() or $('a').map((_, a) => a.href).toArray() Expected output should be array of strings but Typescript compiler won't allow cast from HTMLElement[] to string[]. any[] used to work fine with additional casting. Please, reconsider. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11098 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADhFUd7XF2ZYxPxKVQ2QKo6j7opLixK9ks5rgyotgaJpZM4J3ZMY> .

[dedocibula]

Yeah, functionality-wise these notations are definitely equivalent. I just wanted to reiterate on the fact that this change was motivated by JQuery doc for get(void), yet JQuery doc for map suggests that the above should work, even highlighting it as an example.

[LOZORD]

Good point! In that case, maybe we could have a type parametrized (e.g. JQueryArrayWrapper<T>) version of toArray that returns the correct result type (e.g. T[]).

…

On Mon, Feb 27, 2017 at 4:55 PM Andrej Galad ***@***.***> wrote: Yeah, functionality-wise these notations are definitely equivalent. I just wanted to reiterate on the fact that this change was motivated by JQuery doc for get(void) <https://api.jquery.com/get/>, yet JQuery doc for map <http://api.jquery.com/map/> suggests that the above should work, even highlighting it as an example. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11098 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADhFUQGTY4GJ3v9szYVtcYFx9iA2aM0wks5rg0ZYgaJpZM4J3ZMY> .

[dedocibula]

Not sure if I understood your parametrized idea but I think there are essentially 2 possible approaches.

1. Making toArray() and both get() methods generic

   • this will essentially require to always specify return type, obviously not super convenient - not specifying means {} a.k.a. Object which is typically wrong

2. Having generic JQuery interface

   • this is obviously way more work as it essentially requires changes throughout the entire jquery.d.ts. It will be necessary to analyze all possible return values for JQueryStatic () function declarations but in the end you end up with map() method equivalent to that of arrays in lib.d.ts and both toArray() and get() get solved automatically.

[LOZORD]

We might be able to be lazier (approach 1) and take advantage of default type parameters. From my understanding, JQuery acts as a wrapper around HTMLElements (hence my belief in toArray(): HTMLElement[]), but as you pointed out, there are counterexamples. So by default, JQuery(Static) would be parametrized by HTMLElement[], but calling map<string> for example, would change toArray() to return string[].

[dedocibula]

Yeah, was about to write about the default types. For backwards compatibility I would maybe suggest reverting back to any[] and once TS 2.3 is released switch to toArray<T = any>() and get<T = any>(), or toArray<T = HTMLElement>() and get<T = HTMLElement>().

[LOZORD]

TS 2.3 is out!

I can make the updates soon.