sinonjs/fake-timers

Type definitions introduced in v7 are too strict

peterjuras opened this issue · 3 comments

What did you expect to happen?

The type definitions that have been introduced in v7 appear to be too strict and don't allow users to leave out properties that have default values.

As an example, the following code currently throws an error when using it with TypeScript:

import FakeTimers from "@sinonjs/fake-timers";

FakeTimers.install({
  now: 1
});

FakeTimers.install();

This is because the type definitions do not have question marks for optional properties or arguments. Function parameters and object properties can be marked as optional using a question mark, e.g.:

function example(requiredParam1: { requiredProp1: number; optionalProp1?: number }, optionalParam1?: number) {
  // ...
}

How to reproduce

You can check out the following codesandbox: https://codesandbox.io/s/headless-lake-jq53p?file=/src/index.ts

Quickest way to get this fixed is through a tiny PR with some ?s. You seem to know what is required ... ;)

It will not work. Those type definitions is totally broken )))
What install returns? The Clock object that can be used as a controller for "fake" time in tests. Clock has various functions for advance time: tick, runAll, etc... and to control "fake" time state: reset, setSystemTime... but nothing here )))

Currently it has:

{
    now: number;
    timeouts: {};
    Date: any;
    loopLimit: number;
}

Commit 0d09382 states:

In order to improve the experience of TypeScript users, we are compiling
.d.ts files from the JSDoc and distributing them with the package

But in reality it makes the experience of TypeScript users much worse. Just compare generated from JSDoc types with manual typings from DefinitelyTyped: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/sinonjs__fake-timers/index.d.ts
Before this change, we (TypeScript users) simply npm i -D @types/sinonjs__fake-timers and it was enough. Now we will need to make not trivial configuration to guide typescript to ignore native typings and use those from @types/sinonjs__fake-times...

And no, fix for this not "tiny PR"... )))

I fully support the idea to have typings with code in the same package, but not broken typings please!

@vovkasm We suggest users to stick with version 6 if they want to use the DT types. If you can help us out in improving them that would be great! Would you happen to know if there is a way of configuring TS to use the types from DT instead of the bundled ones? If so, we could add that to our docs (ref #355).