Skip to content

generic type parameters, for generic call expressions, not reduced to canonical form, causing type inference malfuction and errors #56702

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
craigphicks opened this issue Dec 7, 2023 · 7 comments
Labels
Needs More Info The issue still hasn't been fully clarified

Comments

@craigphicks
Copy link

craigphicks commented Dec 7, 2023

πŸ”Ž Search Terms

  • type reduction fails
  • generic function return type inference fails

Couldn't find any issues that match this one sufficiently.

πŸ•— Version & Regression Information

This is the behavior in every version I tried up to 5.3.2.

⏯ Playground Link

playground

πŸ’» Code

import { default as $ } from 'jquery';
import JQuery from 'jquery';

const t1: string | JQuery<HTMLElement> | undefined = $(); // no error

const t2: ((string | JQuery<HTMLElement>) & (string | JQuery<HTMLElement> | undefined)) | undefined = $(); // error
//    ~~
// Type 'JQuery<string | HTMLElement>' is not assignable to type '((string | JQuery<HTMLElement>) & (string | JQuery<HTMLElement> | undefined)) | undefined'.
//  Type 'JQuery<string | HTMLElement>' is not assignable to type 'JQuery<HTMLElement>'.
//    Type 'string | HTMLElement' is not assignable to type 'HTMLElement'.
//      Type 'string' is not assignable to type 'HTMLElement'.(2322)

const t11: string | JQuery<HTMLElement> = $(); // no error

const t12: (string | JQuery<HTMLElement>) & (string | JQuery<HTMLElement> | undefined) = $(); // error
//    ~~
// Type 'JQuery<string | HTMLElement>' is not assignable to type 'string | JQuery<HTMLElement> | (string & JQuery<HTMLElement>) | (JQuery<HTMLElement> & string)'.
//  Type 'JQuery<string | HTMLElement>' is not assignable to type 'JQuery<HTMLElement>'.

const JQuery_string = JQuery<string>; // error
// Type 'string' does not satisfy the constraint 'Element'.(2344)
// Type 'string' does not satisfy the constraint 'HTMLElement'.(2344)
// Type 'string' does not satisfy the constraint 'PlainObject<any>'.(2344)

πŸ™ Actual behavior

  1. Even though the type declarations for t1 and t2 are the same (after reduction), only t2 shows an error. The same relationship exists between t11 and t12.

  2. The return type of assigned to t1 is string | JQuery<HTMLElement> | undefined.

πŸ™‚ Expected behavior

  1. t1 and t2 should have the same error status. Likewise for t11 and t12.

  2. The jQuery function $() has a generic signature <TElement=HTMLElement>() JQuery<TElement>. So the return type assigned to t1 should be JQuery<string> | JQuery<HTMLElement> | JQuery<undefined>.

  3. As shown at the bottom of the code, JQuery<string> is judged to be an invalid instantiation of JQuery. If so, the lines with t1 and t2 should both be errors, but they are not. However, the actual declaration for the interface JQuery seems to be

interface JQuery<TElement = HTMLElement> extends Iterable<TElement>

which actually doesn't look as though it should constraint the domain of JQuery to HTMLElement, so perhaps JQuery<string> should not be an error? Furthermore, from within a DefinitelyTyped/semantic-ui test file, JQuery<string> does not incur an error.

Additional information about the issue

Origin of strange declaration type

I explain where the weird declaration types (e.g. (string | JQuery<HTMLElement>) & (string | JQuery<HTMLElement> | undefined) ) came from.

The following is quoted and condensed from filles under DefinitelyTyped/semantic-ui-search

import { default as $ } from 'jquery';

interface _Impl {
    stateContext: string | JQuery;
};

type Param =
    & (
        | Pick<_Impl, "stateContext">
    )
    & Partial<Pick<_Impl, keyof _Impl>>;

The type of Param["stateContext"] is indeed the weird type:

type StateContext = Param["stateContext"];
//   ^?  (string | JQuery<HTMLElement>) & (string | JQuery<HTMLElement> | undefined)

The error can then be incurred as follows:

const p: Param = {
    stateContext: $()
//  ~~
// Type 'JQuery<string | HTMLElement>' is not assignable 
// to type '(string | JQuery<HTMLElement>) & (string | JQuery<HTMLElement> | undefined)'.(2322)
}

playground

Relation to issue #56013 and pull #56373

The submitted pull pull #56373 for the bug in issue #56013 resulted in similar errors to this report. That's because the bug #56013 is due to improper caching of generic call expression arguments, resulting in the call expression not be re-evaluated when the context changed. When the fix allows that later evaluation, those latent errors appear in the existing test code under DefinitelyTyped/semantic-ui.

An obvious "patchy" fix would be to write new code to force reduction just in time at the critical point - but it could be called multiple times. A better fix would be to do the reduction once when the declaration is parsed - but there is likely some reason it wasn't done that way. Some feedback from the original designers would be helpful.


UPDATE:

Stand alone code with pseudo jQuery

interface JQuery<TElement = HTMLElement> {
    something: any;
    // Change `[Symbol.iterator]` to `other` and the error goes away
    [Symbol.iterator]: () => {
        // Change `next` to `foo` and the error goes away
        next(): {
            value: TElement;
        }
        | {
            done: true;
            value: any;
        };
    }
}

declare function jQuery<TElement = HTMLElement>(): JQuery<TElement>;

const t11: string | JQuery<HTMLElement> = jQuery(); // no error

const t12: (string | JQuery<HTMLElement>) & (string | JQuery<HTMLElement> | undefined) = jQuery(); // error
//    ~~
// Type 'JQuery<string | HTMLElement>' is not assignable to type '(string | JQuery<HTMLElement>) & (string | JQuery<HTMLElement> | undefined)'.
//   Type 'JQuery<string | HTMLElement>' is not assignable to type 'JQuery<HTMLElement>'.ts(2322)

const ta: string  = jQuery(); // error
//                  ?^ jQuery<HTMLElement>(): JQuery<HTMLElement>
const tb: JQuery<HTMLElement>  = jQuery(); // no error
//                               ^? jQuery<HTMLElement>(): JQuery<HTMLElement>
const tc: string & JQuery<HTMLElement>  = jQuery(); // error
//                                        ^? // jQuery<HTMLElement>(): JQuery<HTMLElement>
const td: JQuery<HTMLElement> & string  = jQuery(); // error
//                                        ?^ // jQuery<string>(): JQuery<string>

playground

Debugging analyis using the above code for the psuedo-JQuery

In the declaration

t12: (string | JQuery<HTMLElement>) & (string | JQuery<HTMLElement> | undefined) = jQuery()

the type

(string | JQuery<HTMLElement>) & (string | JQuery<HTMLElement> | undefined)

is reduced to union of these 4 types

  • string
  • JQuery<HTMLElement>
  • string & JQuery<HTMLElement>
  • JQuery<HTMLElement> & string

inferTypes tries to infer TElement in

<TElement = HTMLElement>jQuery(): JQuery<TElement>

by independently inferring for TElement from each of the 4 types above,
to get the following candidates:

  • source: string, target:JQuery<TElement>
    • candidate for TElement: (none)
  • source: JQuery<HTMLElement>, target:JQuery<TElement>
    • candidate for TElement: HTMLElement
  • source: string & JQuery<HTMLElement>, target:JQuery<TElement>
    • candidate for TElement: HTMLElement (repeat, ignored)
  • source: JQuery<HTMLElement> & string, target:`JQuery``
    • candidate for TElement: string

Then it takes the union string | HTMLElement so the final return type is JQuery<string | HTMLElement>.

The most noticeable problem is the result for:

  • source: JQuery<HTMLElement> & string, target:JQuery<TElement>
    • candidate for TElement: string

That happens because inferTypes calls getApparentType(JQuery<HTMLElement> & string)
which promotes string to String.

Then JQuery<HTMLElement> & String is object whose properties are compared to the properties of JQuery<TElemet>.

They both share [[Symbol.iterator]] as a property the the properties

  • source: (() => { next(): { done: true; value: any; } | { value: HTMLElement; }; }) & (() => IterableIterator<string>)
  • target: "() => { next(): { done: true; value: any; } | { value: TElement; }; }"

become a valid source-target pair for inference in the function

function inferFromSignatures(source: Type, target: Type, kind: SignatureKind) {
     const sourceSignatures = getSignaturesOfType(source, kind);
     const sourceLen = sourceSignatures.length;
     if (sourceLen > 0) {
          // We match source and target signatures from the bottom up, and if the source has fewer signatures
          // than the target, we infer from the first source signature to the excess target signatures.
          const targetSignatures = getSignaturesOfType(target, kind);
          const targetLen = targetSignatures.length;
          for (let i = 0; i < targetLen; i++) {
              const sourceIndex = Math.max(sourceLen - targetLen + i, 0);
              inferFromSignature(getBaseSignature(sourceSignatures[sourceIndex]), getErasedSignature(targetSignatures[i]));
          }
     }
}
  • Here the source intersection is treated as an array of signatures.
  • Notice that the source signatures are of length 2, but the target signature is of length 1.
  • The code matches each of the target signatures to some source signature, but because there are more source signatures than target signature, the first source signature is ignored.

Each match potentially contributes a candidates. In this case the deep match stack is as follows:

    • (...args: [] | [undefined]) => IteratorResult<string, any>
    • () => { done: true; value: any; } | { value: TElement; }
    • IterableIterator<string>
    • { next(): { done: true; value: any; } | { value: TElement; }; }
    • IteratorYieldResult<string>
    • { value: TElement; }
    • string
    • TElement

and so string gets added as an inferred candidate for TElement.

There are a couple of remarkable things going on here.

One remarkable thing

Using the the return type of the objects iterator for to infer the typeArgument for the object itself.
By coincide String and JQuery both have iterators, but the inference seems to be an extraordinary overreach.

  • Suggestion: Resolve string & <SomeObject> (SomeObject excepting Enum) to never in getReducedType. In the rare cases where String & <SomeObject> is actually intended to represent their common properties, String can be explicitly specified instead of string.
    • Test: Ran runtests to test this suggestion.
    • Result:
      • It does eliminate the bug noted in this post.
      • However there are 44 tests errors, many of which were cases of type abstractions such as "tagged strings" which have no relation to actual JS runtime types, and could be replaced with some other tagging method such as WeakMap or using a straightforward interface TaggedString<T> {string:string, tag:T}. Overloading the & symbol, which already has a specific meaning in flow and inference, to represent other concepts is not a clean design, so TypeScript should not be obliged to support it.

Another remarkable thing, that does not directly cause the main topic bug

There are a lot assumptions in inferFromSignatures, and because of that these two inferences give difference results;

  • source: string & JQuery<HTMLElement>, target:JQuery<TElement>
    • candidate for TElement: HTMLElement (repeat, ignored)
  • source: JQuery<HTMLElement> & string, target:JQuery<TElement>
    • candidate for TElement: string

The result should not depend upon the order around the & symbol. (However, changes to this "remarkable thing" would not prevent the bug which is the main topic of this post.)

  • Suggestion: If the source and target don't share the same symbol and thus share the same overloads, then in if a full NxM comparison of signatures can not be made, it might be better to do nothing than risk unpredictable behavior. Instead, detect potential inferences from tsserver, and offer to fill in corresponding template constraints as an auto-completion. What I mean is not the final type (which is obviously may be undetermined), but the location in terms of object member path.
@craigphicks craigphicks reopened this Dec 7, 2023
@craigphicks craigphicks changed the title declared variable types not always reduced - can cause type inference malfuction on initializer multiple problems with jQuery() infering its return types given an type argument that needs reduction Dec 7, 2023
@fatcerberus
Copy link

Wow, jQuery is still a thing?

@craigphicks
Copy link
Author

craigphicks commented Dec 7, 2023

@fatcerberus -

The TypeScript CI tests includes DefinitelyTyped which includes semantic-ui which depends on jQuery.

I submitted a fix for #56013 in pull #56373.

The bug in #56013 is improperly caching values of generic call expressions, preventing later correct reevaluation of those call expressions with different generic contexts.

I just so happens that the bug fix #56373 triggered correct reevaluations that revealed the bug in this current bug report.
The error reports occurred in the typescript CI tests for DefinitelyTypes/semantics-ui - you can see the list of errors here.

Just to be clear, this bug report #56702 does not use the code in pull #56373 at all, it is only using already released versions.

I've analyzed this pre-existing bug and condensed it down a simple example.

Maybe the original title was better? changed.

@craigphicks craigphicks changed the title multiple problems with jQuery() infering its return types given an type argument that needs reduction generic type parameters, for generic call expressions, not reduced to canonical form, causing type inference malfuction and error Dec 7, 2023
@craigphicks craigphicks changed the title generic type parameters, for generic call expressions, not reduced to canonical form, causing type inference malfuction and error generic type parameters, for generic call expressions, not reduced to canonical form, causing type inference malfuction and errors Dec 7, 2023
@fatcerberus
Copy link

@craigphicks To be clear, my comment was a joke - jQuery is very old (2006) and it’s rare that JS middleware remains relevant (let alone actively maintained) for that long! πŸ˜„

@craigphicks
Copy link
Author

@fatcerberus - npm jQuery still seems to have million of downloads, whereas semantic-ui is only the thousands, and it is particular usage of $() by semantic-ui in which the latent TypeScript bug is revealed as shown here.

@RyanCavanaugh RyanCavanaugh added the Needs More Info The issue still hasn't been fully clarified label Dec 8, 2023
@RyanCavanaugh
Copy link
Member

We need a repro of this that doesn't depend on something external as large as jquery

@craigphicks
Copy link
Author

craigphicks commented Dec 10, 2023

@RyanCavanaugh - added as an update to original post. Note the accompanying debugger analysis in case it is helpful.

@craigphicks
Copy link
Author

I am closing this because I am adding a little more debugging analysis. When finished I will repost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs More Info The issue still hasn't been fully clarified
Projects
None yet
Development

No branches or pull requests

3 participants