[jquery] Remove type parameter from JQueryStatic

[leonard-thieu]

The type parameter on JQueryStatic does not improve type safety and actually reduces type safety in certain scenarios. This PR removes the type parameter on JQueryStatic and replaces it with member-level type parameters where appropriate.

Previously, the following code was not possible.

let jSvgLineElement2: JQuery<SVGLineElement> = $('.mysvgline');

$('.mysvgline') would use the default type parameter HTMLElement and would not be assignable to jSvgLineElement2 without a double assertion even though the return JQuery object could contain SVGLineElement objects at runtime. This change now makes it possible with the following syntax.

let jSvgLineElement2: JQuery<SVGLineElement> = $<SVGLineElement>('.mysvgline');

@HolgerJeromin

---

Please fill in this template.

• Use a meaningful title for the pull request. Include the name of the package modified.
• Test the change in your own code. (Compile and run.)
• Add or edit tests to reflect the change. (Run with npm test.)
• Follow the advice from the readme.
• Avoid common mistakes.
• Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If changing an existing definition:

• Provide a URL to documentation or source code which provides context for the suggested changes: <>
• Increase the version number in the header if appropriate.
• If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.

[typescript-bot]

@leonard-thieu Thank you for submitting this PR!

🔔 @borisyankov @choffmeister @Steve-Fenton @Diullei @tasoili @jasons-novaleaf @seanski @Guuz @ksummerlin @basarat @nwolverson @derekcicerone @AndrewGaspar @seikichi @benjaminjackman @s093294 @JoshStrobl @johnnyreilly @DickvdBrink @King2500 - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

[typescript-bot]

Since you're a listed owner and the build passed, this PR is fast-tracked. A maintainer will merge shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

[HolgerJeromin]

This is intuitive but wrong.

The target property can be the element that registered for the event or a descendant of it.

So probably it is HTMLElement or SVGElement. The test should be

myDoc.on('click', (evt) => {
    let target = evt.target;
    // $ExpectType Document|HTMLElement
    target;
});

Perhaps the typing should be

on(events: string,
        handler: JQuery.EventHandler<TElement | HTMLElement> | JQuery.EventHandlerBase<any, JQuery.Event<TElement>> | false): this;

But the following code should compile, as in reality a mouse interaction event will not have a target with, too. But this does not compile...

myDoc.on('click', (evt: JQuery.Event<HTMLElement|SVGElement>) => {
    let target = evt.target;
    // $ExpectType HTMLElement|SVGElement
    target;
});

And thanks a lot for the notification :-)

[leonard-thieu]

This has been brought up before in #17377. There's a couple of solutions to the problem but none of them are particularly clean.

First off, let's keep in mind that there are 4 target properties on JQuery.Event.

currentTarget

The current element within the event bubbling phase.

delegateTarget

The element that the handler was attached to. With the way the declarations are currently defined, this property is the only one that has the correct type.

relatedTarget

Another DOM element involved in the event. Set for mouseout and mouseover events.

target

The element that initiated the event. This can be the element that registered the event (delegateTarget) or a descendant.

---

Solution 1

The current solution. All target properties have the same generic type. This works well when TTarget is some base type (e.g. HTMLElement or SVGElement), you're only working with 1 DOM, and you don't need to access properties of more specific types. It otherwise produces incorrect types or types that are too general.

Solution 2a

Set the target properties to a base type. EventTarget would be the correct base type but wouldn't be very useful. This leads to correct typing but requires type assertions to be practical.

Solution 2b

Set the target properties to any. This throws away type safety and autocomplete but is the most flexible.

Solution 3

Introduce type parameters for the other target properties. This allows the user to have the most accurate typings but is a bit unwieldy. This would introduce up to 3 more type parameters on top of the 2 existing. Ordering of the parameters is also significant. They should be arranged to minimize the number of parameters that need to be defined in common cases.

In addition to this, it should also keep in mind additional type parameters that could be introduced to fix the issue of properties being marked optional on JQuery.Event when they are known to exist.

---

Of all these, I prefer Solution 3 for its accuracy. My concern is that mistakes in the implementation could mean a lot of breaking changes for consumers in order to fix them. I would prefer a more graceful solution.

[niikoo]

This commit are not compatible with many other typings, with messages like:
All declarations of 'JQuery' must have identical type parameters.

Also, the comment style is not playing well with vscode:

     * @see \`{@link https://api.jquery.com/find/ }\`
     * @since 1.0
     * @since 1.6

Can some of you look at this, and possible fix or revert this pr? @sandersn @leonard-thieu @holgeradam @Diullei @borislavjivkov @basarat @johnnyreilly
If it there are many breaking changes, I would suggest that it should have been a bigger version bump.
I will use the old version until it's resolved.

[leonard-thieu]

This commit are not compatible with many other typings, with messages like:
All declarations of 'JQuery' must have identical type parameters.

Which typings? All current versions of typings from DefinitelyTyped should be compatible.

Also, the comment style is not playing well with vscode:

Could you elaborate on what you mean (include a screenshot)?

If it there are many breaking changes, I would suggest that it should have been a bigger version bump.

@types/* packages do not follow semantic versioning. Instead, their major and minor version numbers match the version of the library they represent. I recommend locking @types/* packages to a specific version for your needs.

[niikoo]

I will update all our business typings then.