nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23.27k stars 2.32k forks source link

Editors are unable to properly locate types for spec files #816

Closed stijnvn closed 5 years ago

stijnvn commented 5 years ago

Current Behavior

Editors use the next highest tsconfig.json to provide intellisense and thus are unable to grab the appropriate types.

Expected Behavior

Types are attainable by editors/tsserver to properly provide intellisense in the workspace.

Background

The constraints are the following:

Jest, Jasmine, and Chai (through cypress) all define global typings (expect, describe, it) which conflict with each other a. The only way to workaround here is to delete the typings you do not want. b. This is most likely not the approach we want to take here VSCode (tsserver) uses the next highest tsconfig.json to provide intellisense in a .ts file. a. One fix for this, we could generate a tsconfig.json along with tsconfig.app.json/tsconfig.lib.json/tsconfig.e2e.json and tsconfig.spec.json. That file would only be used by the editor and not targetted by any tools. However, I realize that is a lot of configuration. b. The other option is to use /// <reference types="..." at the top of each file. This is a more granular approach and basically allows using specified types in the particular file. This would have to be added to every file (possibly even application code). We could generate this based on the project where code is being generated but you would have to maintain this when manually creating files (jest projects get a reference to jest types, karma projects get references to jasmine types). We have seen large monorepos use both solutions and are not sure which one to go with for the Nx workspace. You can use either one of the "fixes" above as you wish. We would like to hear what the community thinks is a better fix.

Original Issue

Visual Studio Code detects Jasmine instead of Jest.

When I try to use the toBeObservable matcher, I get the following error message:

[ts] Property 'toBeObservable' does not exist on type 'Matchers<...>'

The problem seems to be related with how Visual Studio Code resolves type definitions. It does not use the tsconfig.spec.json but still seems to use the imports defined in test-setup.ts. As a workaround I have added an import for Jest on top of test-setup.ts:

import 'jest';
import 'jest-preset-angular';

For some reason, tests of nx application of the type node-app are correctly type-checked as Jest suites by Visual Studio Code.

Can this be fixed in nx? Or should this be fixed in Visual Studio Code?

skydever commented 5 years ago

hi @stijnvn

somehow the same on my side. I did not use jest specific matchers like said here (and you are using jest-marbles?) that is why I did not run into issues concerning that - expect for the intellisense in vscode. I get jasmine.matchers methods, but when I add the jest import in the test-setup.ts like you did I get jest.Matchers correctly.

I think it is a vscode issue, shouldn't it respect the compilerOptions.types from tsconfig.spec.json? ... from the nearest matching tsconfig that includes the file? that should enable the jest matchers, or do I miss something?

do you have a matcher at hand from pure jest that does not exist in jasmine to play around with?

skydever commented 5 years ago

... and are you using a vscode extension for jest? just wondering, I use vscode-jest, but not very actively at the moment ...

stijnvn commented 5 years ago

I'm not using any jest related vscode extensions.

I'm using jest-marbles. It seems to me that vsode only looks after tsconfig.json files, but not tsconfig.spec.json.

skydever commented 5 years ago

thx for the info about the extension.

I can confirm that vscode seems to only check a tsconfig.json - I created a tsconfig.json next to my spec file and the jest matchers came up, when I rename to tsconfig.spec.json I get the jasmine matchers.

skydever commented 5 years ago

... found this typescript issue, and this vscode issue - vscode is only looking for a tsconfig.json

skydever commented 5 years ago

tried to explicitly import { expect } from 'jest'; in the spec file, but I get [ts] File '.../node_modules/@types/jest/index.d.ts' is not a module When I open the file I see the following errors:

[ts] Definitions of the following identifiers conflict with those in another file: describe, fdescribe, xdescribe, it, fit, xit, beforeEach, afterEach, beforeAll, afterAll, expect, clock, CustomEqualityTester, CustomMatcherFactory, DEFAULT_TIMEOUT_INTERVAL
index.d.ts(14, 1): Conflicts are in this file.

and at the named identifiers

[ts] Duplicate identifier 'beforeAll'.
index.d.ts(14, 18): 'beforeAll' was also declared here.

The link points to @types/jasminewd2/index.d.ts and @types/jasmine/index.d.ts, that also have errors because of duplications.

I thought if I stick to use imports like above this name clashing could not happen. Any thoughts on that?

Bielik20 commented 5 years ago

I am experiencing similar problem but also when running tests I get 'TypeError: expect(...).toBeObservable is not a function'. Do you experience it only with vscode or when running as well?

I have created workspace using nx 6.4.0.

bernardo-cs commented 5 years ago

Webstorm has this issue as well

sandangel commented 5 years ago

you need to uninstall jasmine and @types/jasmine to have correct type in your IDE. or specify the types config in tsconfig.json to include jest

skydever commented 5 years ago

hi @sandangel

you will need @types/jasmine for your e2e tests if you use the jasmine+protractor combination, so uninstalling it may be no solution.

adding "jasmine", "jasminewd2", "jest", "jest-marbles" to the types at tsconfig.json at the root results to listing the jasmine matchers in vscode (there is this conflict), removing "jasmine", "jasminewd2" but keeping "jest", "jest-marbles" results to the jest matchers, but at the e2e test you also get the jest matchers then, but you don't want that because jasmine is used ... vscode is looking at tsconfig.json only, the execution is using the correct tsconfig.spec.json - different behavior here 🤔

sandangel commented 5 years ago

of course, you should not use protractor either. use cypress for that ^^

skydever commented 5 years ago

I am not sure if that would help - you may add the cypress typings at the tsconfig.e2e.json that vscode is ignoring too - not sure about conflicts, did not use cypress before - but yes, I am very interested in doing so and can't wait for #819 from @bcabanes 👍

skydever commented 5 years ago

... but renaming tsconfig.e2e.json to tsconfig.json would help, it is a separate project with one tsconfig only, no explicit need for different names like at the app level for tsconfig.spec.json and tsconfig.app.json ...

skorunka commented 5 years ago

Hi, is really the only solution to rename tsconfig.spec.json to tsconfig.json? It works, but one needs to update angular.json and keep it in mind when generating libs/apps.

FrozenPandaz commented 5 years ago

Hi all,

So we've been looking into and discussing this issue. Thank you for all the debugging in this thread.

I would like to generalize this issue to encompass https://github.com/nrwl/nx/issues/892 as well.

@skydever Was correct with the two typescript issues.

There are two ways to fix and could (but not necessarily) involve some significant changes to the workspace.

Basically, the constraints are the following:

  1. Jest, Jasmine, and Chai (through cypress) all define global typings (expect, describe, it) which conflict with each other a. The only way to workaround here is to delete the typings you do not want. b. This is most likely not the approach we want to take here
  2. VSCode (tsserver) uses the next highest tsconfig.json to provide intellisense in a .ts file. a. One fix for this, we could generate a tsconfig.json along with tsconfig.app.json/tsconfig.lib.json/tsconfig.e2e.json and tsconfig.spec.json. That file would only be used by the editor and not targetted by any tools. However, I realize that is a lot of configuration. b. The other option is to use /// <reference types="..." at the top of each file. This is a more granular approach and basically allows using specified types in the particular file. This would have to be added to every file (possibly even application code). We could generate this based on the project where code is being generated but you would have to maintain this when manually creating files (jest projects get a reference to jest types, karma projects get references to jasmine types).

We have seen large monorepos use both solutions and are not sure which one to go with for the Nx workspace. You can use either one of the "fixes" above as you wish. We would like to hear what the community thinks is a better fix.

stijnvn commented 5 years ago

@FrozenPandaz What about the workaround of adding import 'jest'; to test-setup.ts? I seems to do the job and I haven't seen any side effects.

bernardo-cs commented 5 years ago

@FrozenPandaz appending /// <reference types="..." to test files makes webstorm detect the typings correctly, but it still gets confused on how to run the tests (this might be a webstorm issue though...)

FrozenPandaz commented 5 years ago

@FrozenPandaz What about the workaround of adding import 'jest'; to test-setup.ts? I seems to do the job and I haven't seen any side effects.

This is interesting. Do you know how this works?

There are projects which will not have the test-setup.ts because it does not require it.

goelinsights commented 5 years ago

@FrozenPandaz any insights from how you've set up new apps with cypress in nx 7.0?

Encountered the issue when updating nx of an existing workspace to 7.0.0, so I'd imagine you've made it compatible for new 7.0.0 applications.

FrozenPandaz commented 5 years ago

We've discussed this issue and the better solution seems to be adding tsconfig.json files to each project root.

Adding /// <reference to all spec files is tedious and is too granular as the builders target the whole project.

I will also be writing a page for this in the wiki to explain more. (Link TBD)

Jdruwe commented 5 years ago

Where can I find the documentation?

pkamdem commented 5 years ago

@Jdruwe Here. Still in draft stage though.

micksatana commented 5 years ago

@FrozenPandaz What about the workaround of adding import 'jest'; to test-setup.ts? I seems to do the job and I haven't seen any side effects.

This is interesting. Do you know how this works?

There are projects which will not have the test-setup.ts because it does not require it.

Adding test-setup.ts to the projects which required jest and adding import 'jest'; to test-setup.ts fixed the problem for me. My guess when we import 'jest', typescript use functions in jest module in all the test files, the vscode then see those types correctly.

Stronhold commented 5 years ago

Still same problem using Jasmine + Cypress in VS Code. : Property 'toBeTruthy' does not exist on type 'Assertion'., Any workaround?

danielgeri commented 5 years ago

@Stronhold did you include a tsconfig.json file inside your cypress folder? https://docs.cypress.io/guides/tooling/typescript-support.html#Configure-tsconfig-json

Stronhold commented 5 years ago

Yes, I do have. In fact it is created by the schematic

soupman99 commented 5 years ago

@danielgeri @Stronhold I'm having the same issue. I included the tsconfig in the cypress folder but still get Property 'toBeTruthy' does not exist on type 'Assertion' Any solution?

sanidz commented 5 years ago

@danielgeri @Stronhold I'm having the same issue. I included the tsconfig in the cypress folder but still get Property 'toBeTruthy' does not exist on type 'Assertion' Any solution?

tsconfig inside cypress folder will affect cypres spec files but not existing files for karma unit tests. Because of that I am tinking about creating separate "child" project for cypress only and isolate it with typings so that it cannot affect existing unit test files. Only con is slightly different scripts for cypress open and run from parent project.

AltarBeastiful commented 5 years ago

Could you show us how you did separate cypress and your main project ? Thinking about this too as there seems to be no other solution to have code completion in both cypress and unit tests.

sanidz commented 5 years ago

Could you show us how you did separate cypress and your main project ? Thinking about this too as there seems to be no other solution to have code completion in both cypress and unit tests.

There is also one more "workaround", just add empty import statement inside .spec.ts file like this:

import { } from 'jasmine';

and methods such as .toHaveBeenCalled, .toBeTruthy and all jasmine.Matchers should be recognised.

🔨

AltarBeastiful commented 5 years ago

Thanks sanidz but I still get errors using the empty import.

The only time vscode picks up correctly jasmine is when I uninstall cypress package. I'm considering splitting the projects, but the added complexity is pretty annoying for us.

krokofant commented 5 years ago

@AltarBeastiful I excluded the cypress folder from the root tsconfig.json file and that fixed it for me

AltarBeastiful commented 5 years ago

Adding an exclude works, in my root tsconfig.json I have:

,
  "exclude": [
    "e2e"
  ]

Thanks ! I'll open a PR to cypress doc to document this.

isthatcentered commented 5 years ago

Adding import "cypress" in app/myapp-e2e/src/support/index.ts did the trick for me with phpstorm 🥳

Does it work for anyone else ?

mfechner commented 5 years ago

I created a file in apps/backend/src/test-setup.ts with:

import 'jest';

but it does not help, has there maybe something changed with nx 8.4.3?

danielsmeyer commented 5 years ago

but it does not help, has there maybe something changed with nx 8.4.3?

Fix is working for me in 8.4.3

martijnsenden commented 5 years ago

Adding import "cypress" in app/myapp-e2e/src/support/index.ts did the trick for me with phpstorm 🥳

Does it work for anyone else ?

It does for me (in VSCode). Thanks!

maximedupre commented 4 years ago

Adding an exclude works, in my root tsconfig.json I have:

,
  "exclude": [
    "e2e"
  ]

Thanks ! I'll open a PR to cypress doc to document this.

For those of you trying out this solution, I also had to add exclude: [] to my /e2e/tsconfig.json, or VSC could not resolve the Cypress namespace after restarting VSC (Even restarting the TS server does not show the error).

hamzahsn commented 3 years ago

@maximedupre could you give more context? could you post your tsconfig files for cypress and root project? also I think the solution of @danielgeri should work normally without any issues.

sentient06 commented 2 years ago

This is a nightmare.

wrslatz commented 1 year ago

Just started seeing this issue in Jest test files after upgrading to Cypress 10.

Property 'toBeDefined' does not exist on type 'Assertion'.

Seems this is acknowledged in https://github.com/cypress-io/cypress/issues/22059#issuecomment-1148921141 with a workaround, though I'm not sure why my Jest test files in libraries would be importing Cypress types from a different part of the workspace. This concerns me for performance reasons as well. Might create a separate issue with minimal repro for this one.

EDIT: Maybe Nx could apply changes proposed in https://github.com/cypress-io/cypress-documentation/issues/4559 to resolve the new issues since Cypress 10?

github-actions[bot] commented 1 year ago

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.