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

Possible type definition conflict on @types/chai #1087

Closed
NicholasBoll opened this issue Dec 18, 2017 · 31 comments
Closed

Possible type definition conflict on @types/chai #1087

NicholasBoll opened this issue Dec 18, 2017 · 31 comments
Labels
topic: typescript type: bug type: duplicate This issue or pull request already exists

Comments

@NicholasBoll
Copy link
Contributor

The package.json uses save-exact=true. I didn't really notice this when I added type for Cypress-wrapped types, but it can causes issues if a type definition defined global variables (you can't duplicate global declarations). The issue I'm seeing is in @types/chai.

It looks like Cypress depends on chai@^3.5.0 (Chai is at 4 now - looks like nested was added). A project could also use chai at a different version - normally not a problem as both npm and yarn will embed on conflict detection. But TypeScript won't handle this conflict as gracefully for type definitions. I guess there are a few options to fix this.

Either dependency version string change should prevent yarn or npm from detecting a conflict and flatten the dependency.

Personally I think handling 3.5+ is okay. I can open an issue later. There is a workaround (I'm using yarn, so it will allow resolution overrides).

  • Operating System: N/A
  • Cypress Version: 1.2
  • Browser Version: N/A

Is this a Feature or Bug?

Bug

How to reproduce:

If your application uses TypeScript and you depend on @types/chai in any version other than 4.0.8

Additional Info (images, stack traces, etc)

[CYP] ERROR in ~/{project}/node_modules/cypress/node_modules/@types/chai/index.d.ts
[CYP] (1619,5): error TS2300: Duplicate identifier 'export='.
@bahmutov
Copy link
Contributor

we should not use fuzzy versions, because breaking changes in dependencies can happen. I am also against bringing in yarn - npm is causing enough problems as it is. Can we work around duplicate type declaration by doing something like if undefined chai ... ourselves?

@NicholasBoll
Copy link
Contributor Author

Cypress doesn't have to use yarn. An application using Cypress as a dependency can use yarns resolution feature to work around this issue (by locking both local and transitive versions of @types/chai).

The issues isn't in Cypress type definitions, but rather in having more than 1 @types/chai included in node_modules. Having more than 1 @types/chai is completely up to the package manager and how it decides to do module resolution depending on version matches of local package.json and transitive package.json files.

I agree that fuzzy versions can break things, however, I think fuzzy version strings for type declaration dependencies is preferable to requiring developers to manage transitive dependencies.

@bahmutov
Copy link
Contributor

I see, but still yarn will not solve a problem for anyone using npm. And the problem is really for anyone installing @types/chai AND cypress? Because the TS compiler gets confused and sees duplicate definition. How does it then work for so many people? Everyone is getting then these TS errors for any global object if there are two packages... Hmm, in that case our version of @types/chai can be 4* and hope that no one has 3 in a different place.

@NicholasBoll
Copy link
Contributor Author

I think the issue is deeper than just conflicts in chai. The conflict can happen for any type definition declared on global (jest, mocha, chai, jquery, etc). The issue is Cypress is basically its own environment. The good news is Cypress is meant to run in its own subdirectory of a project (it could be in its own repo, but I like it in the source project).

I think the solution for TypeScript users should be to have their own tsconfig.json where you can set up a more limited environment. I'm experimenting with that right now. It would be good to put these best practices in the TypeScript example with some explanation as to why. If people use that example as a starting point, it will reduce the likelihood that someone will run into this issue.

Proposal:
The idea is you have a tsconfig.json in the cypress folder. This tsconfig.json extends from the tsconfig.json in the base of the project. It has a baseUrl that references the project base path (most likely ../node_modules). It has a typeRoots that picks ../node_modules/cypress/types before ../node_modules/@types to ensure Cypress gets resolved first. Maybe include manually references ../node_modules/cypress/types as well.

I'll try this out and post back.

@bahmutov
Copy link
Contributor

bahmutov commented Dec 19, 2017 via email

@NicholasBoll
Copy link
Contributor Author

I have done some experimentation. Here's what works:

./tsconfig.json

{
  /* ... */
  "types" : ["mocha"] // or "jest" or any other global module your project needs.
  /* ... */
}

./cypress/tsconfig.json

{
  "extends": "../tsconfig",
  "compilerOptions": {
    "baseUrl": "../node_modules" // needed if your tests import npm modules
  },
  "include": [
    "../node_modules/cypress/types",
    "**/*.ts"
  ]
}

Cypress seems to handle TypeScript just fine without any extra configuration, so the cypress/tsconfig.json is just to keep the editor happy.

@NicholasBoll
Copy link
Contributor Author

@kirjai, will that solve your issue?

@bahmutov
Copy link
Contributor

bahmutov commented Dec 19, 2017

I have made https://github.com/bahmutov/test-add-typescript-to-cypress where I have both Jest and Cypress and by having separate tsconfig files the duplicate type definitions are no longer a problem. I think the best solution would be describing this solution in https://docs.cypress.io/faq/questions/general-questions-faq.html

@kirjai
Copy link

kirjai commented Dec 20, 2017

@bahmutov thanks for providing the repo

i checked it out and from what i can see, it works not because of the types isolation, but because of the skipLibCheck flag in the base tsconfig.json. As soon as that flag is set to the default false, npm run check:cy fails with the duplicate definitions error, i'm afraid.

And if I add skipLibCheck in my jest + cypress project, it solves my problem without needing to do anything else.

In my case, i don't see any problems with enabling that flag, so that solution works for me 👍 thanks for taking the time to investigate this :)


However, if separating types with two tsconfig files is still a solution someone is considering, one drawback i can see with that is that by default, if no types option is provided in tsconfig, the typescript compiler will include all the types it can find, however, as soon as types is defined, it will only include those types. So with that solution, developers are stuck having to always keep their types up to date manually which significantly hinders the DX, in my opinion. Just something to consider.

@NicholasBoll
Copy link
Contributor Author

@kirjai You are right that separating types puts some burden on developers. The upside is types only needs to be specified for files with global declarations. Also a way around this when using bundlers is to simply import 'jest' in a spec helper (that will grab the type definitions regardless of types setting in tsconfig.json - easier to maintain possibly?)

From https://www.typescriptlang.org/docs/handbook/tsconfig-json.html#types-typeroots-and-types

Keep in mind that automatic inclusion is only important if you’re using files with global declarations (as opposed to files declared as modules). If you use an import "foo" statement, for instance, TypeScript may still look through node_modules & node_modules/@types folders to find the foo package.

I don't think there is an ideal solution. Cypress uses globals for convenience (no import statements are required) and projects can also have globals for various reasons (legacy or otherwise). The possibility of global conflicts is the reason why modules was introduced. I think documentation can note possible solutions with tradeoffs. Disabling lib checks can have issues as well:

Since declarations in one file can affect type checking in other files, some errors may not be detected when --skipLibCheck is specified. For example, if a non-declaration file augments a type declared in a declaration file, errors may result that are only reported when the declaration file is checked. However, in practice such situations are rare.

I think skipping lib checks is the easiest solution with the least overhead. It also solves issues like chai-enzyme and chai-jquery have conflicting overrides for chai (DefinitelyTyped/DefinitelyTyped#22173). I hesitate to recommend something that disables type checks, but I understand this use-case. I tend to fix the root issue, but that can take a lot more time and effort.

@kirjai
Copy link

kirjai commented Dec 20, 2017

@NicholasBoll thank you, there are some great suggestions there. Totally understand and agree with everything you've said 👍

@bahmutov
Copy link
Contributor

Confirmed, it was skipLibCheck I added to get around Jest "Set" problem. Ok, so the recommended solution is ultimately

{
  "compilerOptions": {
    "skipLibCheck": true
  }
}

in the root tsconfig.json file. Not ideal, but it is hard or impossible to make globals work without clashing.

@kelly-tock
Copy link

why not make @types/mocha a peer dependency or optional dependency? I have cypress and jest, and a third tool that has nothing to do with either but is using typescript and it's picking up on the fact that both are installed.

@in2lag
Copy link

in2lag commented Jan 15, 2019

Hi, can we make this a bit more alive? I see this as big issue and moving mocha to peer dependency as the best option - i just spent non-trivial time now to define the types in app tsconfig, this can introduce issues in the future as project (and number of people) grows. skipLibCheck is really brutal option -> if you have bigger projects, this really becomes handy.

@Vheissu
Copy link

Vheissu commented Jan 16, 2019

I just got bitten by this...

I have Jest for unit tests written in TypeScript, and I just added in Cypress but have my Cypress tests kept as the default .js my Cypress tests work beautifully, however my Jest tests are now throwing errors like Property 'toBeCalled' does not exist on type 'Assertion'. Property 'toEqual' does not exist on type 'Assertion'. Did you mean 'equal'? and a few others.

I have tried everything listed in this issue as well as other issues and repos:

  • Two separate tsconfig.json files
  • skipLibCheck to true (it was already true)
  • Setting types in my root tsconfig.json to be an empty array
  • Adding /// <reference types="Jest" /> to the top of my Jest unit tests
  • Changing the include paths in my root tsconfig.json file

This is a serious issue and it should be reopened @bahmutov

I was able to fix this by doing the following:

{
  "compilerOptions": {
    "types": [],
    "skipLibCheck": true,
    "paths": {
      "chai": ["./noop.d.ts"]
    }
  }
}

In the root you need to create an empty file called noop.d.ts mentioned in the path above.

Super not ideal, but it got me out of a bind short of not using Cypress at all.

pierrebeitz added a commit to dcos/dcos-ui that referenced this issue Feb 23, 2019
cypress ships mocha, which declares e.g. `describe` on a top level, as does
jest. unfortunately their types are incompatible, forcing us to either resort to
whitelisting types (which is tedious) or blindly trusting .d.ts-files.

to learn more, you might want to look here:
cypress-io/cypress#1087
pierrebeitz added a commit to dcos/dcos-ui that referenced this issue Feb 25, 2019
…est types

we now trust .d.ts-files blindly, mostly because the mocha shipped by cypress
conflicts with jest otherwise. an alternative approach would be to whitelist
@types per `"types": ["jest", "graphql", ...],`, but as this introduces a
high chance of forgetting to add new libs here, we stick with skipLibCheck.

-------------------

to learn more, you might want to look here:
cypress-io/cypress#1087
pierrebeitz added a commit to dcos/dcos-ui that referenced this issue Feb 27, 2019
we now trust .d.ts-files blindly, mostly because the mocha shipped by cypress
conflicts with jest otherwise. an alternative approach would be to whitelist
@types per `"types": ["jest", "graphql", ...],`, but as this introduces a
high chance of forgetting to add new libs here, we stick with skipLibCheck.

-------------------

to learn more, you might want to look here:
cypress-io/cypress#1087
pierrebeitz added a commit to dcos/dcos-ui that referenced this issue Feb 27, 2019
we now trust .d.ts-files blindly, mostly because the mocha shipped by cypress
conflicts with jest otherwise. an alternative approach would be to whitelist
@types per `"types": ["jest", "graphql", ...],`, but as this introduces a
high chance of forgetting to add new libs here, we stick with skipLibCheck.

-------------------

to learn more, you might want to look here:
cypress-io/cypress#1087
@maxime1992
Copy link

Why is this issue closed? Doesn't feel like it's solved, at all.

@bahmutov
Copy link
Contributor

bahmutov commented Mar 1, 2019

We are trying to move all @types definitions into our bundle, so this work is done in #3425

pierrebeitz added a commit to dcos/dcos-ui that referenced this issue Mar 1, 2019
we now trust .d.ts-files blindly, mostly because the mocha shipped by cypress
conflicts with jest otherwise. an alternative approach would be to whitelist
@types per `"types": ["jest", "graphql", ...],`, but as this introduces a
high chance of forgetting to add new libs here, we stick with skipLibCheck.

-------------------

to learn more, you might want to look here:
cypress-io/cypress#1087
@jennifer-shehane jennifer-shehane added the type: duplicate This issue or pull request already exists label Mar 1, 2019
@NicholasBoll
Copy link
Contributor Author

@Vheissu You did a lot of things that should work. Be sure to restart your TS language service after each change as it is sometimes not picked up. In VS Code it is "Restart TS" something in the command palette.

This is a hard problem and I'm glad the Cypress team is working to fix the issue and reduce the developer setup burden.

juliangieseke pushed a commit to dcos/dcos-ui that referenced this issue Mar 4, 2019
we now trust .d.ts-files blindly, mostly because the mocha shipped by cypress
conflicts with jest otherwise. an alternative approach would be to whitelist
@types per `"types": ["jest", "graphql", ...],`, but as this introduces a
high chance of forgetting to add new libs here, we stick with skipLibCheck.

-------------------

to learn more, you might want to look here:
cypress-io/cypress#1087
@jdtzmn
Copy link

jdtzmn commented Mar 4, 2019

None of the solutions which @Vheissu described were working for me, including their last one. The thing that finally got my project working was adding the include tag:

{
  "compilerOptions": {
    ...
  },
  "include": [
    "./node_modules/@types/jest"
  ],
  ...
}

Not sure why none of the other solutions didn't work for my project, but just thought I would put this here in case somebody else is having this issue as well.

@jennifer-shehane
Copy link
Member

This PR will likely fix these issues without needing the workaround #3425

@aniketishere
Copy link

Worked for me by adding
"compilerOptions": { "skipLibCheck": true }

Atticus29 added a commit to Atticus29/dataJitsu that referenced this issue Oct 1, 2019
…yment of a firebase cloud function. Had to add skipLibCheck : true to main tsconfig.ts file for success per cypress-io/cypress#1087
@Czar-Ec
Copy link

Czar-Ec commented Nov 5, 2019

Not sure if still relevant but I came across this problem and fixed it via adding:
"compilerOptions": { "typeRoots": [ "node_modules/cypress/types" ] }
to the cypress tsconfig

then
"compilerOptions": { "typeRoots": [ "node_modules/@types" ] }
to the root tsconfig as well as removing the types property

@joanllenas
Copy link

joanllenas commented Nov 12, 2019

Yet another fix, in case someone finds it useful.

I had confict issues with global types.
In my case, I was using Angular (v8), which uses jasmine and karma by default, and I added Cypress (v3.6.1).

The issue I had was that after installing Cypress, suddenly my Angular specs started complaining because the expect() type was coming from chai instead of jasmine.

My (hacky) fix was adding this to the main tsconfig.json file.

{
    (...)
    "include": [
        "src",
        "node_modules/cypress"
    ],
    "exclude": [
        "node_modules/cypress"
    ]
    (...)
}

I did that because you can only exclude what has been included.

@AlirezaInGitHub
Copy link

I ended up with this solution:
1- ✨ Install cypress-jest-adapter
2- 🖊 Add "include": ["./node_modules/jest"] to the root tsconfig.json. No additional tsconfig is needed.
3- 🧪 Add import {} from "cypress"; to cypress tests files.
4- 🧪 If I ever need to have jest tests inside cypress folder, I add import "cypress-jest-adapter"; to the test files.
🎉 Jest tests in other directories don't need any modification.

@soywod
Copy link

soywod commented Dec 24, 2019

Adding "types": ["@types/jest", "cypress"] in this specific order fixed the problem for me.

@mi-mazouz
Copy link

Still having this issue, what is supposed to solve it?

@jennifer-shehane
Copy link
Member

@mi-mazouz Yes, this was meant to be fixed as part of #3425 in 3.2.0.

If you're experiencing a bug similar to this in Cypress, please open a new issue with a fully reproducible example that we can run. There may be a specific edge case with the issue that we need more detail to fix.

@mazjindeel
Copy link

@jdtzmn solution worked for me

@eric-burel
Copy link
Contributor

eric-burel commented Apr 27, 2020

The issue specifically reappeared after latest update to 4.4.1, is there any change that could have affected this? Will try to produce a repro later.
Edit: adding cypress to excluded folders of my global tsconfig did the job. But I am not sure why the issue appeared suddenly, there might be an underlying root cause.

@maury91
Copy link

maury91 commented Dec 7, 2022

Unfortunately, I didn't have the option to exclude Cypress from my compiled files ( I need to import the run method ).

I solved by overriding the types that Cypress was using to override my types:

declare namespace Chai {
    export type ExpectStatic = jest.Expect;
}

declare namespace Mocha {
    export type TestFunction = jest.It;
}

@toto1384
Copy link

toto1384 commented Dec 9, 2022

It's sad that this has no universal fix. Tried all the solutions and none worked

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: typescript type: bug type: duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests