From bac2d1db5698d9ca8ef8cbf2823f2aadc36606e8 Mon Sep 17 00:00:00 2001 From: Leonard Thieu Date: Sun, 17 Jun 2018 18:02:23 -0400 Subject: [PATCH] [jquery] Fix return types for `JQuery.map`, `JQueryStatic.map`, and `JQueryStatic()`. (#26524) * [jquery] Fix return type of `JQuery.map`. The type parameter of the JQuery object returned from `JQuery.map` depends on the return values of the callback. Previously, the type parameter was based on the object it was called on. This fix requires dropping constraints on `JQuery`, `JQueryStatic`, `EventHandler`, and `EventHandlerBase` as `JQuery.map` may return `JQuery` objects that contain non-`Node` values. * [jquery] Fix return type of `JQueryStatic.map`. * [jquery] Declare type for `this` in callback for `JQueryStatic.map`. * [jquery] Fix return type for `JQueryStatic()`. * [bootstrap] Match change to `JQuery` interface. * [flight] Fix test failure due to change in `@types/jquery`. * [materialize-css] Fix test failure due to change in `@types/jquery`. * [select2] Match changes to `@types/jquery` interfaces. * [jquery] Fix error due to breaking change in TypeScript lib declarations. (cherry picked from commit 2506245) --- types/bootstrap/index.d.ts | 2 +- types/flight/flight-tests.ts | 2 +- types/jquery/index.d.ts | 62 +++-- types/jquery/jquery-tests.ts | 233 ++++++++++++++++-- types/jquery/test/example-tests.ts | 2 +- .../test/jquery-slim-window-module-tests.ts | 2 +- .../jquery/test/jquery-window-module-tests.ts | 2 +- types/jquery/tslint.json | 1 + .../materialize-css/test/inputfields.test.ts | 2 +- types/select2/index.d.ts | 6 +- 10 files changed, 273 insertions(+), 41 deletions(-) diff --git a/types/bootstrap/index.d.ts b/types/bootstrap/index.d.ts index 2f43555a73..bba7b71c59 100755 --- a/types/bootstrap/index.d.ts +++ b/types/bootstrap/index.d.ts @@ -354,7 +354,7 @@ export type TooltipEvent = "show.bs.tooltip" | "shown.bs.tooltip" | "hide.bs.too // -------------------------------------------------------------------------------------- declare global { - interface JQuery extends Iterable { + interface JQuery { alert(action?: "close" | "dispose"): this; button(action: "toggle" | "dispose"): this; diff --git a/types/flight/flight-tests.ts b/types/flight/flight-tests.ts index bd2f22d1c3..dd21f32f48 100644 --- a/types/flight/flight-tests.ts +++ b/types/flight/flight-tests.ts @@ -1,6 +1,6 @@ declare var el: Element; -declare var els: Element[]; +declare var els: HTMLElement[]; declare var mixinFn: Function; function TestComponent() { diff --git a/types/jquery/index.d.ts b/types/jquery/index.d.ts index b0838f9a8d..79525510e9 100644 --- a/types/jquery/index.d.ts +++ b/types/jquery/index.d.ts @@ -40,7 +40,7 @@ type _Event = Event; // Used by JQuery.Promise3 and JQuery.Promise type _Promise = Promise; -interface JQueryStatic { +interface JQueryStatic { /** * @see {@link http://api.jquery.com/jquery.ajax/#jQuery-ajax1} * @deprecated Use jQuery.ajaxSetup(options) @@ -137,23 +137,53 @@ interface JQueryStatic { // HACK: The discriminator parameter handles the edge case of passing a Window object to JQueryStatic. It doesn't actually exist on the factory function. (window: Window, discriminator: boolean): JQueryStatic; /** + * Return a collection of matched elements either found in the DOM based on passed argument(s) or created + * by passing an HTML string. + * + * @param element_elementArray A DOM element to wrap in a jQuery object. + * An array containing a set of DOM elements to wrap in a jQuery object. + * @see {@link https://api.jquery.com/jQuery/} + * @since 1.0 + */ + (element_elementArray: T | ArrayLike): JQuery; + /** + * Return a collection of matched elements either found in the DOM based on passed argument(s) or created + * by passing an HTML string. + * + * @param selection An existing jQuery object to clone. + * @see {@link https://api.jquery.com/jQuery/} + * @since 1.0 + */ + (selection: JQuery): JQuery; + /** + * Accepts a string containing a CSS selector which is then used to match a set of elements. + * * Creates DOM elements on the fly from the provided string of raw HTML. * * Binds a function to be executed when the DOM has finished loading. * * @param selector_object_callback A string containing a selector expression - * A DOM element to wrap in a jQuery object. - * An array containing a set of DOM elements to wrap in a jQuery object. - * A plain object to wrap in a jQuery object. - * An existing jQuery object to clone. + * A string of HTML to create on the fly. Note that this parses HTML, not XML. * The function to execute when the DOM is ready. * @see {@link https://api.jquery.com/jQuery/} * @since 1.0 + */ + (selector_object_callback: JQuery.Selector | JQuery.htmlString | ((this: Document, $: JQueryStatic) => void)): JQuery; // tslint:disable-line:unified-signatures + /** + * Return a collection of matched elements either found in the DOM based on passed argument(s) or created by passing an HTML string. + * + * @param object A plain object to wrap in a jQuery object. + * @see {@link https://api.jquery.com/jQuery/} + * @since 1.0 + */ + (object: T): JQuery; + /** + * Returns an empty jQuery set. + * + * @see {@link https://api.jquery.com/jQuery/} * @since 1.4 */ - (selector_object_callback?: JQuery.Selector | JQuery.htmlString | JQuery.TypeOrArray | JQuery | - JQuery.PlainObject | Window | - ((this: Document, $: JQueryStatic) => void)): JQuery; + (): JQuery; /** * A multi-purpose callbacks list object that provides a powerful way to manage callback lists. * @@ -659,7 +689,7 @@ interface JQueryStatic { * @see {@link https://api.jquery.com/jQuery.map/} * @since 1.0 */ - map(array: T[], callback: (elementOfArray: T, indexInArray: number) => R): R[]; + map(array: T[], callback: (this: Window, elementOfArray: T, indexInArray: number) => JQuery.TypeOrArray | null | undefined): TReturn[]; /** * Translate all items in an array or object to new array of items. * @@ -671,7 +701,7 @@ interface JQueryStatic { * @see {@link https://api.jquery.com/jQuery.map/} * @since 1.6 */ - map(obj: T, callback: (propertyOfObject: T[K], key: K) => R): R[]; + map(obj: T, callback: (this: Window, propertyOfObject: T[K], key: K) => JQuery.TypeOrArray | null | undefined): TReturn[]; /** * Merge the contents of two arrays together into the first array. * @@ -3098,7 +3128,7 @@ interface JQueryStatic { when(...deferreds: any[]): JQuery.Promise; } -interface JQuery extends Iterable { +interface JQuery extends Iterable { /** * A string containing the jQuery version number. * @@ -4253,7 +4283,7 @@ interface JQuery extends Iterable * @see {@link https://api.jquery.com/map/} * @since 1.2 */ - map(callback: (this: TElement, index: number, domElement: TElement) => any | any[] | null | undefined): this; + map(callback: (this: TElement, index: number, domElement: TElement) => JQuery.TypeOrArray | null | undefined): JQuery; /** * Bind an event handler to the "mousedown" JavaScript event, or trigger that event on an element. * @@ -6363,7 +6393,9 @@ declare namespace JQuery { }; // Writable properties on XMLHttpRequest - interface XHRFields extends Partial> { } + interface XHRFields extends Partial> { + msCaching?: string; + } } interface Transport { @@ -7922,9 +7954,9 @@ declare namespace JQuery { // endregion - interface EventHandler extends EventHandlerBase> { } + interface EventHandler extends EventHandlerBase> { } - interface EventHandlerBase { + interface EventHandlerBase { // Extra parameters can be passed from trigger() (this: TContext, t: T, ...args: any[]): void | false | any; } diff --git a/types/jquery/jquery-tests.ts b/types/jquery/jquery-tests.ts index 47a85b80d2..89dfabb97e 100644 --- a/types/jquery/jquery-tests.ts +++ b/types/jquery/jquery-tests.ts @@ -42,7 +42,7 @@ function JQueryStatic() { // $ExpectType JQuery $([new HTMLElement()]); - // $ExpectType JQuery + // $ExpectType JQuery<{ foo: string; hello: string; }> $({ foo: 'bar', hello: 'world' }); // $ExpectType JQuery @@ -58,6 +58,39 @@ function JQueryStatic() { // $ExpectType JQuery $(); + + // https://github.com/DefinitelyTyped/DefinitelyTyped/issues/19597#issuecomment-378218432 + function issue_19597_378218432() { + let myDiv = $(document.createElement('div')); // gets default jQuery 8-/ + // $ExpectType JQuery + myDiv; + myDiv.on('click', (evt) => { + let target = evt.target; // HTMLElement + // $ExpectType HTMLDivElement + target; + }); + let myDiv1 = $(document.createElement('div')); // expected 0-2 Arguments but got 1. huh? + // let myDiv2 = $(document.createElement('div')); // expected 0-1 Arguments but got 2. huh? + let myForcedDiv: JQuery = $(document.createElement('div')) as any; + myForcedDiv.on('click', (evt) => { + let target = evt.target; // HTMLDivElement + // $ExpectType HTMLDivElement + target; + }); + let myDoc = $(document); // gets default jQuery + // $ExpectType JQuery + myDoc; + myDoc.on('click', (evt) => { + let target = evt.target; // HTMLElement + // $ExpectType Document + target; + }); + let myDocForced: JQuery = $(document); // type HTMLElement is not assignable to Type Document + let myWindow = $(window); // gets default jQuery + // $ExpectType JQuery + myWindow; + let myWindowForced: JQuery = $(window); // type Window does not satisfy contraint Node + } } function ajaxSettings() { @@ -699,7 +732,9 @@ function JQueryStatic() { function map() { // $ExpectType number[] - $.map([1, 2, 3], (elementOfArray, indexInArray) => { + $.map([1, 2, 3], function (elementOfArray, indexInArray) { + // $ExpectType Window + this; // $ExpectType number elementOfArray; // $ExpectType number @@ -708,11 +743,49 @@ function JQueryStatic() { return 200 + 10; }); + // $ExpectType number[] + $.map([1, 2, 3], function (elementOfArray, indexInArray) { + // $ExpectType Window + this; + // $ExpectType number + elementOfArray; + // $ExpectType number + indexInArray; + + return [200, 10]; + }); + + // $ExpectType (number | null)[] + $.map([1, 2, 3], function (elementOfArray, indexInArray) { + // $ExpectType Window + this; + // $ExpectType number + elementOfArray; + // $ExpectType number + indexInArray; + + return [200, 10, null]; + }); + + // $ExpectType (number | undefined)[] + $.map([1, 2, 3], function (elementOfArray, indexInArray) { + // $ExpectType Window + this; + // $ExpectType number + elementOfArray; + // $ExpectType number + indexInArray; + + return [200, 10, undefined]; + }); + // $ExpectType (false | 1)[] $.map({ myProp: true, name: 'Rogers', - }, (propertyOfObject, key) => { + }, function (propertyOfObject, key) { + // $ExpectType Window + this; // $ExpectType string | boolean propertyOfObject; // $ExpectType "myProp" | "name" @@ -725,6 +798,67 @@ function JQueryStatic() { return false; } }); + + // $ExpectType (string | number | boolean)[] + $.map({ + myProp: true, + name: 'Rogers', + }, function (propertyOfObject, key) { + // $ExpectType Window + this; + // $ExpectType string | boolean + propertyOfObject; + // $ExpectType "myProp" | "name" + key; + + return [propertyOfObject, 24]; + }); + + // $ExpectType (false | 1)[] + $.map({ + myProp: true, + name: 'Rogers', + anotherProp: 70, + }, function (propertyOfObject, key) { + // $ExpectType Window + this; + // $ExpectType string | number | boolean + propertyOfObject; + // $ExpectType "myProp" | "name" | "anotherProp" + key; + + switch (key) { + case 'myProp': + return 1; + case 'name': + return false; + } + + return null; + }); + + // $ExpectType (false | 1)[] + $.map({ + myProp: true, + name: 'Rogers', + anotherProp: 70, + }, function (propertyOfObject, key) { + // $ExpectType Window + this; + // $ExpectType string | number | boolean + propertyOfObject; + // $ExpectType "myProp" | "name" | "anotherProp" + key; + + switch (key) { + case 'myProp': + return 1; + case 'name': + return false; + } + + return undefined; + }); } function merge() { @@ -2041,7 +2175,7 @@ function JQuery() { function ajax() { function ajaxComplete() { - // $ExpectType JQuery + // $ExpectType JQuery $(document).ajaxComplete(function(event, jqXHR, ajaxOptions) { // $ExpectType Document this; @@ -2057,7 +2191,7 @@ function JQuery() { } function ajaxError() { - // $ExpectType JQuery + // $ExpectType JQuery $(document).ajaxError(function(event, jqXHR, ajaxSettings, thrownError) { // $ExpectType Document this; @@ -2075,7 +2209,7 @@ function JQuery() { } function ajaxSend() { - // $ExpectType JQuery + // $ExpectType JQuery $(document).ajaxSend(function(event, jqXHR, ajaxOptions) { // $ExpectType Document this; @@ -2091,7 +2225,7 @@ function JQuery() { } function ajaxStart() { - // $ExpectType JQuery + // $ExpectType JQuery $(document).ajaxStart(function() { // $ExpectType Document this; @@ -2101,7 +2235,7 @@ function JQuery() { } function ajaxStop() { - // $ExpectType JQuery + // $ExpectType JQuery $(document).ajaxStop(function() { // $ExpectType Document this; @@ -2111,7 +2245,7 @@ function JQuery() { } function ajaxSuccess() { - // $ExpectType JQuery + // $ExpectType JQuery $(document).ajaxSuccess(function(event, jqXHR, ajaxOptions, data) { // $ExpectType Document this; @@ -5898,8 +6032,9 @@ function JQuery() { } function contents() { - // $ExpectType JQuery - $('p').contents(); + // TODO: Flaky test due to type ordering. + // // $ExpectType JQuery + // $('p').contents(); } function end() { @@ -6131,7 +6266,7 @@ function JQuery() { } function map() { - // $ExpectType JQuery + // $ExpectType JQuery $('p').map(function(index, domElement) { // $ExpectType HTMLElement this; @@ -6143,7 +6278,7 @@ function JQuery() { return 'myVal'; }); - // $ExpectType JQuery + // $ExpectType JQuery $('p').map(function(index, domElement) { // $ExpectType HTMLElement this; @@ -6155,7 +6290,7 @@ function JQuery() { return ['myVal1', 'myVal2']; }); - // $ExpectType JQuery + // $ExpectType JQuery $('p').map(function(index, domElement) { // $ExpectType HTMLElement this; @@ -6164,10 +6299,10 @@ function JQuery() { // $ExpectType HTMLElement domElement; - return null; + return ['myVal1', 'myVal2', null]; }); - // $ExpectType JQuery + // $ExpectType JQuery $('p').map(function(index, domElement) { // $ExpectType HTMLElement this; @@ -6176,8 +6311,72 @@ function JQuery() { // $ExpectType HTMLElement domElement; - return undefined; + return ['myVal1', 'myVal2', undefined]; }); + + // $ExpectType JQuery + $('p').map(function(index, domElement) { + // $ExpectType HTMLElement + this; + // $ExpectType number + index; + // $ExpectType HTMLElement + domElement; + + let value: string; + + if (index % 2 === 0) { + return null; + } + + value = 'myVal'; + + return value; + }); + + // $ExpectType JQuery + $('p').map(function(index, domElement) { + // $ExpectType HTMLElement + this; + // $ExpectType number + index; + // $ExpectType HTMLElement + domElement; + + let value: string; + + if (index % 2 === 0) { + return undefined; + } + + value = 'myVal'; + + return value; + }); + + // // $ExpectType JQuery + // $('p').map(function(index, domElement) { + // // $ExpectType HTMLElement + // this; + // // $ExpectType number + // index; + // // $ExpectType HTMLElement + // domElement; + // + // return null; + // }); + + // // $ExpectType JQuery + // $('p').map(function(index, domElement) { + // // $ExpectType HTMLElement + // this; + // // $ExpectType number + // index; + // // $ExpectType HTMLElement + // domElement; + // + // return undefined; + // }); } function slice() { diff --git a/types/jquery/test/example-tests.ts b/types/jquery/test/example-tests.ts index 728a42b3d6..f725cb2bcb 100644 --- a/types/jquery/test/example-tests.ts +++ b/types/jquery/test/example-tests.ts @@ -3428,7 +3428,7 @@ function examples() { function map_0() { $('p') .append($('input').map(function() { - return $(this).val(); + return $(this).val() as string; }) .get() .join(', ')); diff --git a/types/jquery/test/jquery-slim-window-module-tests.ts b/types/jquery/test/jquery-slim-window-module-tests.ts index b47707cb23..bf4331d824 100644 --- a/types/jquery/test/jquery-slim-window-module-tests.ts +++ b/types/jquery/test/jquery-slim-window-module-tests.ts @@ -1,5 +1,5 @@ import jq = require('jquery/dist/jquery.slim'); const $window = jq(window); -// $ExpectType JQuery +// $ExpectType JQuery $window; diff --git a/types/jquery/test/jquery-window-module-tests.ts b/types/jquery/test/jquery-window-module-tests.ts index 6bc5486fb9..72cb44c5aa 100644 --- a/types/jquery/test/jquery-window-module-tests.ts +++ b/types/jquery/test/jquery-window-module-tests.ts @@ -1,7 +1,7 @@ import jq = require('jquery'); const $window = jq(window); -// $ExpectType JQuery +// $ExpectType JQuery $window; class CanvasLayersDirective { diff --git a/types/jquery/tslint.json b/types/jquery/tslint.json index deae83dc66..d1d7bd0a0a 100644 --- a/types/jquery/tslint.json +++ b/types/jquery/tslint.json @@ -14,6 +14,7 @@ "no-empty-interface": false, "no-misused-new": false, "no-object-literal-type-assertion": false, + "no-redundant-jsdoc-2": false, "no-unnecessary-generics": false, "no-unnecessary-qualifier": false, "no-unnecessary-type-assertion": false, diff --git a/types/materialize-css/test/inputfields.test.ts b/types/materialize-css/test/inputfields.test.ts index ecde571537..15ea61c1ee 100644 --- a/types/materialize-css/test/inputfields.test.ts +++ b/types/materialize-css/test/inputfields.test.ts @@ -1,6 +1,6 @@ import * as materialize from "materialize-css"; -const elem = document.querySelector('.whatever')!; +const elem = document.querySelector('.whatever') as HTMLElement; M.textareaAutoResize(elem); M.textareaAutoResize($(elem)); diff --git a/types/select2/index.d.ts b/types/select2/index.d.ts index a0c1ce3b1d..240b4b7f25 100644 --- a/types/select2/index.d.ts +++ b/types/select2/index.d.ts @@ -26,7 +26,7 @@ export type JQueryAjaxSettingsBase = /** * Same as jQuery v3 `JQuery.EventHandlerBase`. */ -export type JQueryEventHandlerBase = +export type JQueryEventHandlerBase = (this: TContext, t: T, ...args: any[]) => void | false; /** @@ -221,7 +221,7 @@ export interface Options { +export interface Select2Plugin { amd: { require: Require; }; defaults: { @@ -252,7 +252,7 @@ export interface Select2Plugin { } declare global { - interface JQuery { + interface JQuery { select2: Select2Plugin; data(key: "select2"): Select2;