Definitelytyped: @types/cheerio pulls unconditionally @types/node

[sormy]

@types/enzyme pulls @types/cheerio that pulls @types/node using triple slash notation that is not compatible with web platform and can't be disabled using types property in tsconfig.json causing react tests using enzyme to fail due to TS issues caused by incompatibility of setTimeout(), setInterval() etc signatures between web and node platform.

This is regression introduced in this commit: https://github.com/DefinitelyTyped/DefinitelyTyped/commit/602e04583e74ac8eaead02a1269a9a33192e1c9a#diff-b6bce2a5160083400070377ac11edbb8

The workaround is to lock dependency on version @types/[email protected].

Authors: @blittle, @wmaurer, @umarniz, @LiJinyao, @chennakrishna8, @AzSiAz, @nwtgck

[sormy]

I propose to move nodejs bindings into @types/cheerio-node so users will have an option what typings to install depending on platform they are targeting

[sormy]

I would vote to recall @types/[email protected] until the issue will be fixed

[sormy]

Any update?
Authors: @blittle, @wmaurer, @umarniz, @LiJinyao, @chennakrishna8, @AzSiAz, @nwtgck

[LiJinyao]

I have a concern about duplication, since if we split into two projects, we need to sync change to both of them. Is there a better way?

[AzSiAz]

Let's not split in two project, I don't think that's a good idea.

[sormy]

So, what is the proposed solution for the problem?

React typescript testing using enzyme pulls cheerio (dependency of enzyme), cheerio pulls @types/node that conflicts with typescript web typings for things like setTimeout, clearTimeout, setInterval, clearInterval etc.

The only workaround for this problem for now is to lock dependency "@types/cheerio": "=0.22.10" but it is already implicit forking of @types/cheerio on two versions =0.22.10 that doesn't pull @types/node and >0.22.10 that pulls @types/node.

[vkrol]

The only workaround for this problem for now is to lock dependency "@types/cheerio": "=0.22.10"

We are using this solution :(
I think that this is the best solution:

I propose to move nodejs bindings into @types/cheerio-node so users will have an option what typings to install depending on platform they are targeting

[sormy]

Any update?
Authors: @blittle, @wmaurer, @umarniz, @LiJinyao, @chennakrishna8, @AzSiAz, @nwtgck

[AzSiAz]

I would like a repo maintainer or another authors response before someone try to go down this road and publish, but personally I still think @types/cheerio-node is a bad idea

[sormy]

@AzSiAz Could you please explain why is it a bad idea? Especially due to lack of other ideas.

[AzSiAz]

Well because the name of the lib is cheerio not cheerio-node or cheerio-browser and no one will try to install @types/cheerio-node. This and the fact that people are already using @types/cheerio in their package.json.

We need a different solution, I will try to look into it this week if I can.

@sandersn Sorry to bother you but I am not an expert in typescript... Do you have some pointer on how to fix this problem without creating another package ?
Someone posted something in the merge request introducing this regression: possible solution
Thanks

[sandersn]

The possible solution, faking a forward declaration, sounds like it is worth a try. It's slightly less safe for non-node users since they can pass string | {} for the parameter in question. You'll have to weigh the value of that vs the value of string | Buffer for node users.

Forking cheerio/cheerio-browser is the only reliable way to do this, but it's a lot of trouble to update long dependency chains. From its documentation, cheerio seems node-centric, but it's possible that most of its users come through react and enzyme. In case of a fork, cheerio should be the more commonly used one and either cheerio-browser or cheerio-node should be the less commonly used one.

In general, @types packages don't have a good solution for this kind of optional dependency. Forward declarations are probably the right solution, but the fake forward declarations that people write now aren't as safe as real ones, baked into the language, would be.

[vkrol]

Related:
https://github.com/Microsoft/TypeScript/issues/18588
https://github.com/microsoft/TypeScript/issues/32273

[sormy]

Another workaround - add typing file that will override node's declaration:

File: types/node.d.ts:

// avoid: "Augmentations for the global scope can only ..."
export {}

// `enzyme` testing library requires `@types/enzyme` that pulls `@types/cheerio`.
// `@types/cheerio` >0.22.10 pulls `@types/node` that breaks the build.

// workaround:
// override node's timers that break type checking for code that is targeting browser platform
declare global {
  function clearInterval(handle?: number): void;
  function clearTimeout(handle?: number): void;
  function clearImmediate(handle?: number): void;
  function setInterval(handler: TimerHandler, timeout?: number, ...arguments: any[]): number;
  function setTimeout(handler: TimerHandler, timeout?: number, ...arguments: any[]): number;
  function setImmediate(handler: any, ...args: any[]): number;
}

File: tsconfig.json:

{
  "compilerOptions": {
    ...
    "skipLibCheck": true
    ...
  },
  "include": [
    ...
    "types"
  ]
}

[Grunet]

I don't think this is currently a significant use case to consider, but I was trying Cheerio out with Deno and ran into this as well. Following @sormy's original workaround, I pinned a dependence on the version of the index.d.ts file before the commit they mentioned, which seemed to work for my playground use case's needs.

// @deno-types="https://raw.githubusercontent.com/DefinitelyTyped/DefinitelyTyped/deab75bde42b5a82aeb951f5a2edaa09922853f4/types/cheerio/index.d.ts"
import cheerio from "https://dev.jspm.io/[email protected]";