Skip to content
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

Regression: Rest parameter typed as union of tuple types behaves differently in 4.3 #44093

Closed
dragomirtitian opened this issue May 14, 2021 · 3 comments Β· Fixed by #44122
Closed
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@dragomirtitian
Copy link
Contributor

Bug Report

πŸ”Ž Search Terms

tuples rest parameter regression

πŸ•— Version & Regression Information

  • This changed between versions 4.2 and 4.3-beta

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

class EventEmitter<ET> {
    // works with overloads
    // off<K extends keyof ET>(...args: [K, unknown]): void
    // off<K extends keyof ET>(...args: [K | unknown]): void
    off<K extends keyof ET>(...args: [K, unknown] | [K | unknown]):void {}

    
}
function once<ET, T extends EventEmitter<ET>>(
    emittingObject: T,
    eventName: keyof ET,
    callback: (event: unknown) => void,
): void {
    emittingObject.off(eventName, callback); /* fails*/
    
    // type assertion works as well
    emittingObject.off(eventName as keyof ET, callback);
    
}

πŸ™ Actual behavior

The call emittingObject.off(eventName, callback); fails with Argument of type '[string | number | symbol, (event: unknown) => void]' is not assignable to parameter of type '[unknown] | [keyof ET, unknown]'.

πŸ™‚ Expected behavior

The call should succeed as it did in 4.2

@RyanCavanaugh
Copy link
Member

Trimming off the last little bit of fat:

class EventEmitter<ET> {
    off<K extends keyof ET>(...args: [K, number] | [unknown, string]):void {}
}
function once<ET, T extends EventEmitter<ET>>(emittingObject: T, eventName: keyof ET): void {
    // Fail
    emittingObject.off(eventName, 0);
    // Pass (?!)
    emittingObject.off(eventName as typeof eventName, 0);
}

It looks like something's gone wrong here where we construct a contextual type for argument position 0 (correct) and then decide to use the presence of that to devolve eventName from keyof ET to its constraint string | number | symbol (bad). But it's something specific to the tuple syntax, since equivalentish forms like off<K extends keyof ET>(args: K | boolean, a2: string | number):void {} don't trigger this.

@ahejlsberg
Copy link
Member

This is caused by #43183. The issue is that the contextual type of the first argument to off is K | unknown, which ends up reducing to just unknown. Since unknown contains no generic types we think it is safe to reduce keyof ET to its constraint string | number | symbol which would be a candidate for narrowing by CFA. The fix is to ensure K | unknown doesn't get reduced. We do that elsewhere for contextual types, just not in the particular scenario of arguments to rest parameters with union types.

@dragomirtitian
Copy link
Contributor Author

@DanielRosenwasser @RyanCavanaugh This does not appear fixed in 4.3.2 (Playground Link) but works in nightly (Playground Link). Will there be a 4.3.3 where this will be included ?

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants