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

[Bug]: "expect" package now imports @types/node causing web projects to break #14418

Closed
octogonz opened this issue Aug 16, 2023 · 4 comments · Fixed by #14385
Closed

[Bug]: "expect" package now imports @types/node causing web projects to break #14418

octogonz opened this issue Aug 16, 2023 · 4 comments · Fixed by #14385

Comments

@octogonz
Copy link

octogonz commented Aug 16, 2023

Version

>= 29.6.0

Steps to reproduce

Try to compile a TypeScript web project with Jest tests and code like this:

function asyncLoadStyles(): number {
  return setTimeout(() => {
    _themeState.runState.flushTimer = 0;
    flush();
  }, 0);
}

Expected behavior

It should compile because the compiler provides this definition:

lib.dom.d.ts

declare function setTimeout(handler: TimerHandler, timeout?: number, ...arguments: any[]): number;

Actual behavior

It fails to compile because expect now also brings in @types/node which has this conflicting definition:

globals.d.ts

declare function setTimeout(callback: (...args: any[]) => void, ms?: number, ...args: any[]): NodeJS.Timeout;

The error:

src/index.ts:224:3 - error TS2322: Type 'Timeout' is not assignable to type 'number'.

224   return setTimeout(() => {
      ~~~~~~~~~~~~~~~~~~~~~~~~~
225     _themeState.runState.flushTimer = 0;
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
226     flush();
    ~~~~~~~~~~~~
227   }, 0);
    ~~~~~~~~

Real world failing build:
microsoft/rushstack#3949
https://github.com/microsoft/rushstack/actions/runs/5873236375/job/15926196196?pr=3949

Additional context

This is a regression introduced by PR #14139

https://github.com/jestjs/jest/pull/14139/files#diff-1f480f1988b7f343469bb28b165fad08b4cadc37da379e655f2317d3c5f048b3

Why it's wrong: Jest itself is a Node.js application, but the TypeScript typings for Jest's test API should not assume that the unit tests are targeting Node.js runtime. For example if the unit tests are for a web application, then the setTimeout() declaration may be different from the Node.js environment.

Environment

System:
    OS: Windows 10 10.0.22621
    CPU: (14) x64 AMD Ryzen 7 3700X 8-Core Processor
  Binaries:
    Node: 16.15.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - C:\Program Files\nodejs\yarn.CMD
    npm: 8.11.0 - C:\Program Files\nodejs\npm.CMD
    pnpm: 7.16.1 - C:\Program Files\nodejs\pnpm.CMD
@octogonz
Copy link
Author

After discussing this further: Jest does actually run your web application in a Node.js runtime environment (using mocks like jsdom to make it work). And unit tests often choose to import from APIs such as fs or path, for example to assist with loading of test assets. If so, then the test code may need to import @types/node, which unavoidably will bring in an incompatible typing for setTimeout(). In that situation, we recommend the workaround of writing window.setTimeout() instead of setTimeout(), which works because @types/node global declarations do not apply to the window object.

However, if a project's unit tests do NOT choose to import from @types/node, then it is not good for the typings of basic APIs like describe() to force @types/node to be imported (via expect). So there really is a significant regression here, which Jest should fix.

@mrazauskas
Copy link
Contributor

Thanks for opening this issue. The problem was reported in #14139 (comment) and the fix is waiting to be merged (see #14385).

@SimenB
Copy link
Member

SimenB commented Aug 21, 2023

https://github.com/jestjs/jest/releases/tag/v29.6.3

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants