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

setTimeout defined in lib.dom.d.ts is not type safe #32186

Open
yw662 opened this issue Jul 1, 2019 · 6 comments
Open

setTimeout defined in lib.dom.d.ts is not type safe #32186

yw662 opened this issue Jul 1, 2019 · 6 comments
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript
Milestone

Comments

@yw662
Copy link

yw662 commented Jul 1, 2019

TypeScript Version: 3.5.2

Search Terms:
settimeout type safe
Code

// A *self-contained* demonstration of the problem follows...
// Test this by running `tsc` on the command-line, rather than through another build tool such as Gulp, Webpack, etc.
const f = (foo: number) => console.log(foo + 1);
setTimeout(f, 0, 'a');

Expected behavior:
It shall not pass thanks to the type error.
Actual behavior:
It passes.
Playground Link:
https://www.typescriptlang.org/play/#code/MYewdgzgLgBAZjAvDAFHEIBcMwFcC2ARgKYBOAlEgHwyiQgA2xAdAyAOZoYwDUMAjOQDcAKAjEoAFQCW+YiFxQ0AGhgAGVQHIAhoULBgAE0PFNWwpuFA
Related Issues:
emm, maybe not.

@fatcerberus
Copy link

Is the reason you're expecting this to fail because you've passed extra parameters to setTimeout that the callback doesn't have in its signature? Because TypeScript by design allows you to use a function type with fewer parameters in place of one that takes more. See, e.g. Array.prototype.map callback.

@kitsonk
Copy link
Contributor

kitsonk commented Jul 1, 2019

I suspect the OP expects that setTimeout to guard against whatever the callback function guards against, as additional parameters are passed to the callback. Because f accepts a single argument of number, the OP expects setTimeout to be something like this:

setTimeout(callback: (foo: number) => void, timeout: number, foo: number): number;

The DOM signatures are based off the WebIDL, which wouldn't be able to model the relationship between the callback signature and the arguments passed to setTimeout. While TypeScript can extract the arguments, I am not sure they can be expressed properly in an generic overloaded signature. I expect it is a current design limitation.

@fatcerberus
Copy link

fatcerberus commented Jul 1, 2019

Oh, I see, the number argument of the callback corresponds with the third parameter of setTImeout in the example, I was thinking that was part of the setTimeout machinery itself. In any case what you describe is perfectly possible to model:

declare function setTimeout<A extends any[]>(callback: (...args: A) => void, timeout: number, ...args: A): number;

IntelliSense will even help you out here!
image

@yw662
Copy link
Author

yw662 commented Jul 1, 2019

@fatcerberus Thanks for the signature and it works. But there is still a problem that, it breaks the ability that setTimeout can use a string as callback, although I am personally happy with that feature.
The original signature is:

declare function setTimeout(handler: TimerHandler, timeout?: number, ...arguments: any[]): number;
type TimerHandler = string | Function;

Btw, this does not only apply to setTimeout, but also setInterval, which can be improved in the same way.

declare function setInterval<A extends any[]>(callback: (...args: A) => void , timeout?: number, ...args: A): number;

@yw662
Copy link
Author

yw662 commented Jul 1, 2019

declare function setTimeout<A extends any[]>(handler: (...args: A) => void, timeout?: number, ...arguments: A): number;
declare function setTimeout(handler: string, timeout?: number, ...arguments: any[]): number;
declare function setInterval<A extends any[]>(handler: (...args: A) => void, timeout?: number, ...arguments: A): number;
declare function setInterval(handler: string, timeout?: number, ...arguments: any[]): number;

That will do it.

So another question:
How should I / Should I make a PR about that ?

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Jul 1, 2019
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jul 1, 2019
@RyanCavanaugh RyanCavanaugh added the Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript label Jul 1, 2019
@ghost
Copy link

ghost commented Mar 14, 2020

There's a PR for this on the TSJS-lib-generator repo: microsoft/TypeScript-DOM-lib-generator#810

Unfortunately, it was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants