Skip to content

jquery has batch-only inference error after re-aliasing support (##42284) #42317

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
sandersn opened this issue Jan 13, 2021 · 3 comments · Fixed by #42365
Closed

jquery has batch-only inference error after re-aliasing support (##42284) #42317

sandersn opened this issue Jan 13, 2021 · 3 comments · Fixed by #42365
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status.

Comments

@sandersn
Copy link
Member

Notably, this does not show up in the editor. I haven't got it to reproduce outside the jquery project either:

  • You can see this error with npm test jquery inside Definitely Typed.
  • Or you can observe that tsc before Support re-aliasing of type alias instantiations #42284 has 12 (expected) errors, whereas tsc afterward has 14 errors.
  • Or you can paste this code into a test file (I used jquery-slim-no-window-module-tests.ts), since it's a smaller repro:
function delegate_0(events: string, handler: JQuery.TypeEventHandler<HTMLElement, any, any, any, string>) {
    delegate(events, handler);
}
declare function delegate<TType extends string>(eventType: TType, handler: JQuery.TypeEventHandler<HTMLElement, any, any, any, TType>): void;

Expected: No error on the argument events

Actual: Error on the argument events: "Argument of type 'string' is not assignable to parameter of type "myEvent"."

It looks like TType is inferred as "myEvent" instead of string. I can't figure out why. TType should be constrained to string | number, since its constraint as written is TType extends keyof TypeToTriggeredEventMap<TDelegateTarget, TData, TCurrentTarget, TTarget> and TypeToTriggeredEventMap has a string index signature (plus a dozen properties or so).

Changing events to "myEvent" gets rid of the error.

@sandersn sandersn added Bug A bug in TypeScript Needs Investigation This issue needs a team member to investigate its status. labels Jan 13, 2021
@ahejlsberg
Copy link
Member

Mmm, nasty little issue. Problem is that the keys we compute for type instantiations need to include both the alias and the alias type arguments. I was only including the alias in the mistaken belief that the rest of the key would represent all necessary information. But it is possible (though clearly very rare) for an alias to have type arguments that map many-to-one to a single type representation--at which point we randomly pick one of them. So, only by including the alias type arguments is the key truly unique.

@ahejlsberg
Copy link
Member

ahejlsberg commented Jan 15, 2021

I the OP example, the last type parameter of JQuery.TypeEventHandler<...> is used in a manner that maps different type arguments to a single type. Thus we started confusing instantiations of the type.

@ahejlsberg
Copy link
Member

Here's a simple repro for the issue:

type Foo<T> = T | string | number;
type Bar<T> = Foo<T> | undefined;

declare let x1: Bar<'a'>;
declare let x2: Bar<'b'>;

With the current nightly build, depending on which variable you hover on first we'll pick one or the other instantiation of Bar and use it for both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants