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

@types/chai conflict with @vitest/expect/dist/chai.d.cts #4688

Open
6 tasks done
antoniosZ opened this issue Dec 6, 2023 · 23 comments
Open
6 tasks done

@types/chai conflict with @vitest/expect/dist/chai.d.cts #4688

antoniosZ opened this issue Dec 6, 2023 · 23 comments

Comments

@antoniosZ
Copy link

Describe the bug

since v1.0.0-beta.1
5996c8c0

I get the following:

../../node_modules/@types/chai/index.d.ts:1:1 - error TS6200: Definitions of the following identifiers conflict with those in another file: Message, ObjectProperty, ChaiPlugin, AssertionArgs, Operator, OperatorComparable, AssertionError, chai

1 declare namespace Chai {
  ~~~~~~~

  ../../node_modules/vitest/node_modules/@vitest/expect/dist/chai.d.cts:16:1
    16 declare namespace Chai {
       ~~~~~~~
    Conflicts are in this file.

../../node_modules/vitest/node_modules/@vitest/expect/dist/chai.d.cts:16:1 - error TS6200: Definitions of the following identifiers conflict with those in another file: Message, ObjectProperty, ChaiPlugin, AssertionArgs, Operator, OperatorComparable, AssertionError, chai

16 declare namespace Chai {
   ~~~~~~~

  ../../node_modules/@types/chai/index.d.ts:1:1
    1 declare namespace Chai {
      ~~~~~~~
    Conflicts are in this file.


Found 2 errors in 2 files.

In case it matters, this is an NPM workspace project.
If I delete the file node_modules/vitest/node_modules/@vitest/expect/dist/chai.d.cts everything compiles successfully like before the upgrade to v1.0.0-beta.1.

Reproduction

install and try to compile without skipLibCheck: true in tsconfig

System Info

System:
    OS: macOS 13.6.1
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.9.0 - ~/.nvm/versions/node/v20.9.0/bin/node
    npm: 10.2.1 - ~/my-project/node_modules/.bin/npm
  npmPackages:
    @vitest/coverage-v8: ^1.0.1 => 1.0.1 
    vite: ^5.0.6 => 5.0.6

Used Package Manager

npm

Validations

@sheremet-va
Copy link
Member

Why do you have @types/chai installed?

@antoniosZ
Copy link
Author

@sheremet-va they are indirect/nested deps

npm list @types/chai
@my-project
  ├─┬ @storybook/test@7.6.3
  │ └── @types/chai@4.3.5
  └─┬ vitest@0.34.6
    ├─┬ @types/chai-subset@1.3.3
    │ └── @types/chai@4.3.5 deduped
    └── @types/chai@4.3.5 deduped

@sheremet-va
Copy link
Member

I wonder if this should be fixed on storybook side - they are using chai to use @vitest/expect (form what I gathered here: storybookjs/storybook#17326). It's no longer needed because we bundle it together with @vitest/expect package because of #2118

We can also try to improve @types/chai package but I have no idea how to reach maintainers there.

@antoniosZ
Copy link
Author

@sheremet-va I removed the @storybook/test package and I still get the same conflict with the following

$npm list @types/chai
@testing-library/jest-dom@6.1.5
    └─┬ vitest@0.34.6
      ├─┬ @types/chai-subset@1.3.3
      │ └── @types/chai@4.3.5 deduped
      └── @types/chai@4.3.5

@antoniosZ
Copy link
Author

https://github.com/testing-library/jest-dom/blob/main/package.json#L104

seems like to resolve this, everyone else has to remove their @types/chai dependency or vitest has to remove their bundled typings of chai...

@sheremet-va
Copy link
Member

https://github.com/testing-library/jest-dom/blob/main/package.json#L104

seems like to resolve this, everyone else has to remove their @types/chai dependency or vitest has to remove their bundled typings of chai...

This is a dev dependency, it's not installed. jest-dom has a peer dependency of Vitest which should just reuse existing one. Try deleting a lockfile and installing everything again.

@antoniosZ
Copy link
Author

@sheremet-va I just created a minimal reproduction...
https://stackblitz.com/edit/vitest-dev-vitest-nowys5?file=package.json
if you run in the terminal tsc you will see the conflict...

@storybook/test for sure creates the conflict

@antoniosZ
Copy link
Author

antoniosZ commented Dec 7, 2023

the way I see this, any other package that has @types/chai as a dependency, will cause a conflict with vitest, just because vitest embeds the chai typings. But I could be wrong in my understanding here.

@antoniosZ
Copy link
Author

one more reproduction, this time with the nightwatch package, to hopefully demonstrate, that the issue IMO is not isolated to @storybook/test...

https://stackblitz.com/edit/vitest-dev-vitest-qwc3pw?file=package.json

@majo44
Copy link

majo44 commented Dec 20, 2023

Hi, we have also this issue.

We have huge monorepo (~400 packages/rush/pnpm), we are on the mocha/chai/sinon, and we made a try to migrate package by package to vitests. In general tests runs well, but compilation (type checking) on packages where we are using the vitest fails with:

../../../../common/temp/node_modules/.pnpm/@types+chai@4.3.9/node_modules/@types/chai/index.d.ts:1:1 - error TS6200: Definitions of the following identifiers conflict with those in another file: Message, ObjectProperty, ChaiPlugin, AssertionArgs, Operator, OperatorComparable, AssertionError, chai

1 declare namespace Chai {
  ~~~~~~~

  ../../../../common/temp/node_modules/.pnpm/@vitest+expect@1.1.0/node_modules/@vitest/expect/dist/chai.d.cts:16:1
    16 declare namespace Chai {
       ~~~~~~~
    Conflicts are in this file.

../../../../common/temp/node_modules/.pnpm/@vitest+expect@1.1.0/node_modules/@vitest/expect/dist/chai.d.cts:16:1 - error TS6200: Definitions of the following identifiers conflict with those in another file: Message, ObjectProperty, ChaiPlugin, AssertionArgs, Operator, OperatorComparable, AssertionError, chai

16 declare namespace Chai {
   ~~~~~~~

  ../../../../common/temp/node_modules/.pnpm/@types+chai@4.3.9/node_modules/@types/chai/index.d.ts:1:1
    1 declare namespace Chai {
      ~~~~~~~
    Conflicts are in this file.


Found 2 errors in 2 files.

Errors  Files
     1  ../../../../common/temp/node_modules/.pnpm/@types+chai@4.3.9/node_modules/@types/chai/index.d.ts:1
     1  ../../../../common/temp/node_modules/.pnpm/@vitest+expect@1.1.0/node_modules/@vitest/expect/dist/chai.d.cts:16

The package itself is not depends on the @types/chai, but probably somewhere deep in the tree we (or 3partpy dep or dev dep) has such dependency/devDependecy, so it is installed in the tree, but .... funny is that the vitests is causing of loading of @types/chai, during the compilation:

tsc --explainFiles says:

../../../../common/temp/node_modules/.pnpm/@types+chai@4.3.9/node_modules/@types/chai/index.d.ts
  Imported via 'chai' from file '../../../../common/temp/node_modules/.pnpm/vitest@1.1.0_@types+node@20.10.3/node_modules/vitest/dist/reporters-O4LBziQ_.d.ts' with packageId '@types/chai/index.d.ts@4.3.9'
  Imported via 'chai' from file '../../../../common/temp/node_modules/.pnpm/vitest@1.1.0_@types+node@20.10.3/node_modules/vitest/dist/index.d.ts' with packageId '@types/chai/index.d.ts@4.3.9'

In vitest 0.34 there was no such problem as vitest used the @types/chai instead of re-declare it internally.

@qraynaud
Copy link

qraynaud commented Apr 8, 2024

Did someone find a workaround for this? This is very cumbersome…

@qraynaud
Copy link

qraynaud commented Apr 8, 2024

Okay, I just found that adding skipLibCheck: true to the tsconfig.json/compilerOptions is doing the trick.

@kasperpeulen
Copy link

Trying to fix this in the next patch release of @storybook/test.

@Abdillah
Copy link

Abdillah commented Jun 5, 2024

This seems to be Vitest issue.

Chai is obviously should not be included along with user facing vitest type. If Vitest has resemblance with Chai, it should be hidden.

@danielrentz
Copy link

I see a similar problem after upgrading vitest from 2.0.4 to 2.0.5

Without having @types/chai installed, I see

node_modules/vitest/dist/chunks/reporters.C_zwCd4j.d.ts:17:23 - error TS7016: Could not find a declaration file for module 'chai'. '.../node_modules/chai/chai.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/chai` if it exists or add a new declaration (.d.ts) file containing `declare module 'chai';`

17 import * as chai from 'chai';
                         ~~~~~~
Found 1 error in node_modules/vitest/dist/chunks/reporters.C_zwCd4j.d.ts:17

After installing @types/chai as suggested, I see

node_modules/@types/chai/index.d.ts:1:1 - error TS6200: Definitions of the following identifiers conflict with those in another file: Message, ObjectProperty, ChaiPlugin, AssertionArgs, Operator, OperatorComparable, AssertionError, chai

1 declare namespace Chai {
  ~~~~~~~

  node_modules/@vitest/expect/dist/chai.d.cts:16:1
    16 declare namespace Chai {
       ~~~~~~~
    Conflicts are in this file.

node_modules/@vitest/expect/dist/chai.d.cts:16:1 - error TS6200: Definitions of the following identifiers conflict with those in another file: Message, ObjectProperty, ChaiPlugin, AssertionArgs, Operator, OperatorComparable, AssertionError, chai

16 declare namespace Chai {
   ~~~~~~~

  node_modules/@types/chai/index.d.ts:1:1
    1 declare namespace Chai {
      ~~~~~~~
    Conflicts are in this file.

Found 2 errors in 2 files.

Errors  Files
     1  node_modules/@types/chai/index.d.ts:1
     1  node_modules/@vitest/expect/dist/chai.d.cts:16

osmestad added a commit to tidal-music/tidal-sdk-web that referenced this issue Aug 14, 2024
osmestad added a commit to tidal-music/tidal-sdk-web that referenced this issue Aug 14, 2024
* Update vitest monorepo to v2

* Remove "--sequence.concurrent" as it caused failure in some packages

* Revert to `vitest` 2.0.4 as 2.0.5 has more Chai problems: vitest-dev/vitest#4688

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Øyvind Smestad <oyvind@tidal.com>
jeremymeng added a commit to Azure/azure-sdk-for-js that referenced this issue Sep 19, 2024
***NO_CI***

- replace version to "~5.6.2"
- rush update
- add `"skipLibCheck": true` for several packages that are affected by vitest-dev/vitest#4688
jeremymeng added a commit to Azure/azure-sdk-for-js that referenced this issue Sep 20, 2024
***NO_CI***

- replace version to "~5.6.2"
- rush update
- add `"skipLibCheck": true` for several packages that are affected by vitest-dev/vitest#4688

- [formrecognizer] help TypeScript to understand with more explicit type

- [schema-registry-{avro,json}] skip lib check due to lru-cache issue with the Iterator TReturn change

isaacs/node-lru-cache#348
@Jym77
Copy link

Jym77 commented Oct 28, 2024

Running into this too.
My use case is that I try to build integrations of my library with various test frameworks (so that end-user can more easily use it).

So, I have a monorepo alfa-integrations where the existing package alfa-chai is importing Chai and its types. I'm trying to create an alfa-vitest package, importing Vitest. But every dependency in a monorepo is hosted in the root node_modules and somehow just having vitest and @types/chai there causes this problem.

@tassoevan
Copy link

Seeing it happen in a codebase with @storybook/test (depends on @vitest/expect) and @types/chai. It broke all assert.isDefined() calls as the function went from being an assertion function to a void-returning function.

@alecgibson
Copy link

alecgibson commented Feb 28, 2025

This is also a problem if you try to use any chai plugins with vitest (we're also trying to migrate from mocha/sinon/chai):

import {chai} from 'vitest';
import chaiDom from 'chai-dom';
import chaiAsPromised from 'chai-as-promised';

chai.use(chaiDom);
chai.use(chaiAsPromised);
❯ npm list @types/chai
@my-project
├─┬ @types/chai-as-promised@8.0.1
│ └── @types/chai@5.0.1 deduped
└─┬ @types/chai-dom@1.11.3
  └── @types/chai@5.0.1 deduped

I don't think it's particularly strange that a chai plugin would include @types/chai.

Going to see if I can work around this by hand-rolling the types for the plugins, but it's not a great consumer experience.

@alecgibson
Copy link

I'm probably missing something, but can't vitest also just depend on @types/chai and let npm do its thing?

@alecgibson
Copy link

Okay one hack around this if you don't want @vitest/expect's augmented chai types (ie if you're migrating from vanilla chai or want full control over your own chai plugins) is to tweak the shipped type definitions with a hideous tsconfig.json hack:

{
  "compilerOptions": {
    "paths": {
      "@vitest/expect": ["./typings/vitest-expect.d.ts"],
    }
  }
}

Where vitest-expect.d.ts looks like this:

export * from '@vitest/expect/dist/index';

Which is basically the same as the shipped type definition, without the augmented chai import.

@hi-ogawa
Copy link
Contributor

hi-ogawa commented Mar 1, 2025

I'm probably missing something, but can't vitest also just depend on @types/chai and let npm do its thing?

@alecgibson One reason I'm aware of is because @types/chai injects global typing Object.should. We have this patched out and copied to @vitest/expect/dist/chai.d.cts. This isn't really ideal, but I guess this approach is chosen as a trade off. If you have any other ideas, please feel free to suggest 🙏

-interface Object {
- should: Chai.Assertion;
-}
+// interface Object {
+// should: Chai.Assertion;
+// }

I think your workaround of directly grabbing @vitest/expect/dist/index seems nice. Thanks for sharing 👍

@alecgibson
Copy link

@hi-ogawa while I agree mutating the global Object is horrible, I'm not (personally) sure that patching the library like this is the "correct" approach, for exactly the above reasons in this issue.

I think I'd expect one of the following:

  • Just accept that patching Object was a design decision made with chai. If you don't agree with upstream decisions,
    don't use chai and use something else instead; or...
  • Fork chai "properly" instead of this patch. Then you can re-release it under your own namespace @vitest/chai which won't clash with existing type definitions. Comes with the downside that existing plugin types won't work, but that's already the case anyway and this way you wouldn't break existing types

Obviously I'm not a maintainer/contributor on this library and wasn't privy to any of these design decisions (so what do I know), but that's what I personally would have done I think.

I guess if you wanted to make this a DefinitelyTyped problem (which it also sort of is), then I'd maybe raise a PR to drop this global augmentation from @types/chai, and move this augmentation into the namespace chai/register-should or something for clients to opt in to these types.

@alecgibson
Copy link

For what it's worth, I've tried opening a PR against @types/chai for this: DefinitelyTyped/DefinitelyTyped#72101

I don't know if it'll get accepted, since it'll be breaking for some users who use chai.should()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests