TypeScript icon indicating copy to clipboard operation
TypeScript copied to clipboard

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

Open yw662 opened this issue 6 years ago • 7 comments

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.

yw662 avatar Jul 01 '19 00:07 yw662

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.

fatcerberus avatar Jul 01 '19 02:07 fatcerberus

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.

kitsonk avatar Jul 01 '19 02:07 kitsonk

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

fatcerberus avatar Jul 01 '19 02:07 fatcerberus

@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 avatar Jul 01 '19 07:07 yw662


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 ?

yw662 avatar Jul 01 '19 07:07 yw662

There's a PR for this on the TSJS-lib-generator repo: https://github.com/microsoft/TSJS-lib-generator/pull/810

Unfortunately, it was closed.

ghost avatar Mar 14 '20 08:03 ghost

This is still a problem even though the underlying root cause seems to have been addressed.

function setTimeout<TArgs extends any[]>(
  callback: (...args: TArgs) => void,
  ms?: number,
  ...args: TArgs
) {
  console.log(ms)
  callback(...args)
}

function foo<TArgs extends any[]>(
  callback: (...args: TArgs) => void,
  ms?: number,
  ...args: TArgs
) {
  console.log(ms)
  callback(...args)
}

function callback(bar: string) {
  console.log(bar);
}

foo(callback, 0); // Error - as expected
setTimeout(callback, 0); // No error

This illustrates the typescript is now capable of correctly checking the type but for some reason, it doesn't do it for setTimeout. Apparently with some lookup issue (It is claimed that the type within the module is being used. But if it is... why the inconsistency?) as a bonus? (And presumably for any similar function declaration in a library?)

Repro: https://github.com/s-h-a-d-o-w/settimeout-repro

s-h-a-d-o-w avatar Oct 16 '24 12:10 s-h-a-d-o-w