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

typescript compilerOptions not reflected #7188

Closed
keeema opened this issue Apr 30, 2020 · 26 comments
Closed

typescript compilerOptions not reflected #7188

keeema opened this issue Apr 30, 2020 · 26 comments
Assignees

Comments

@keeema
Copy link

keeema commented Apr 30, 2020

Cypress TypeScript compilator probably do not reflects some of compilerOptions. In my case it looks like e.g. strict is ignored.

I am sure that my tsconfig.json is the one which is used because e.g. I have forgotten "noEmitHelpers": true and it raised errors so I needed to remove it.

It was working with webpack compiler.

Current behavior:

tsconfig.json:

{
    "compilerOptions": {
        "strict": true,
        "noUnusedLocals": true,
        "noUnusedParameters": true,
        "noImplicitReturns": true,
        "noFallthroughCasesInSwitch": true,
        "noImplicitAny": true,
        "strictPropertyInitialization": true,
        "preserveConstEnums": false,
        "target": "es5",
        "lib": ["es5", "dom", "es2015.core", "es2015.promise", "es2015.iterable", "es2015.collection"],
        "types": ["cypress"],
        "sourceMap": true,
        "removeComments": false
    },
    "include": ["**/*.ts"]
}

test:

describe("example", () => {
    it("example", () => {
        let a: boolean | undefined;
        let b: boolean = a;
        console.log(b);
    });
});

VS Code reflects tsconfig.json and shows error:

Type 'boolean | undefined' is not assignable to type 'boolean'.
Type 'undefined' is not assignable to type 'boolean'.ts(2322)

Cypress run doesn't reflects tsconfig.json and successfully compiles script.

Desired behavior:

Cypress also reflects tsconfig.json and shows error:

Type 'boolean | undefined' is not assignable to type 'boolean'.
Type 'undefined' is not assignable to type 'boolean'.ts(2322)

Versions

4.5.0

@jennifer-shehane jennifer-shehane added topic: typescript v4.4.0 🐛 Issue present since 4.4.0 labels May 1, 2020
@cypress-bot cypress-bot bot added the stage: needs investigating Someone from Cypress needs to look at this label May 1, 2020
@sainthkh
Copy link
Contributor

sainthkh commented May 1, 2020

Checking types slows down the transpilation. That's why Cypress ignores type checks and delegate that to other tools like tsc and VS Code.

It's an intended feature, not a bug.

@keeema
Copy link
Author

keeema commented May 1, 2020

I understand that Cypress wants to be fast. But it in this case it does not make a sense.

1. Type checks are the reason why use TypeScript.
2. I expect that Cypress also uses tsc to compile my .ts files. There is no reason to compile it again with another webpack and tsc. In the result...it would be much slower then the previous solution, when we used just webpack preprocessor and tsc....
3. It should be developer's decision what restrictons are used.
4. I need to see errors from compiler. IDE like VS Code is not enough. If I make a change in one file and it creates an error with some restrictions, which will have impact in another file, then I would not know about it, because IDE just shows errors in opened files. And to be honest... you can also simply dismiss errors from IDE. It is like you don't have TypeScript.

Consider this as a bug, not as a feature, because embedded TypeScript whithout type checks I defined in tsconfig.json is not usable. It has no sense and I still have to setup webpack preprocessor, so finally compile it twice...

@sainthkh
Copy link
Contributor

sainthkh commented May 1, 2020

1. I agree type checks are the reason why we use TypeScript. But tsc can do the job. Cypress doesn't have to do it again.
2. Cypress doesn't use tsc. It uses TypeScript compiler API. And Cypress should bundle its own runner program together with your test code. So, we cannot simply use the code built with your webpack settings.
3. Yes, you're right. Tools should give developers to choose their own restrictions. That's why you can use cypress-webpack-preprocessor or cypress-browserify-preprocessor to control the Cypress preprocessing process.
4. To restrict everyone to obey types, I guess the better way is to use tsc --noEmit and add it to your CI to check it every time you push something to your repo.

@sainthkh sainthkh added stage: awaiting response Potential fix was proposed; awaiting response and removed stage: needs investigating Someone from Cypress needs to look at this labels May 1, 2020
@keeema
Copy link
Author

keeema commented May 1, 2020

As I read about the API in the link above, it allows you to read external tsconfig file... or I think it is also possible to just parse compiler options and provide it to TS API, if Cypress wants to manage the rest of configuration.

For me and I am sure not just for me it would be awesome to not need any other webpack preprocessor.

So what exactly is the problem in allowing this?

@sainthkh
Copy link
Contributor

sainthkh commented May 1, 2020

If type check is built in by default, you always need to wait for 6-10 seconds to run a test suite after each change in files. I don't think it's not a good default option. Right?

@keeema
Copy link
Author

keeema commented May 1, 2020

It is the same time I would wait with webpack-preprocessor plugin - so it is not a problem.
For me getting realtime feedback from type check is more important.

@keeema
Copy link
Author

keeema commented May 1, 2020

You say built in by default ... I think this could be just optional/configurable feature. So apart of this. Is there any other problem?

@sainthkh
Copy link
Contributor

sainthkh commented May 1, 2020

For some people, that time difference is huge.

Without type checking, Cypress is already running after file changes. But with type checking, I need to wait for a few seconds each time to see Cypress working. It's that difference.

The goal of Cypress is to help developers do e2e tests, not checking types real time. And that real time thing is done by editors.

And you want Cypress to find type errors, but it cannot find the type errors across the entire project even if we sacrifice the response time and add type checking. It can just find type errors in the test file you're running and type errors in the file you imported to that test file.

Unfortunately, Cypress TypeScript parser cannot do the job you want.

EDIT: And you're saying making it configurable. That's the job of those preprocessors I mentioned above.

@keeema
Copy link
Author

keeema commented May 1, 2020

For me it would be enough to have type check for current test file and all its dependencies. This is also exactly what the webpack preprocessor does. VS code doesn't show you possible problems in dependcies if you don't open it.

As I said... it could by just optional. Then it would be on preference of each developer - speed vs type safety.

I see embedded TypeScript as awesome feature and as a big step forward. But without such possiblity it has no added value for me.

It would cause hidden problems and postpone tsc check to CI is not solution, it is too late.

@sainthkh
Copy link
Contributor

sainthkh commented May 1, 2020

Just a question: preprocessors can do everything you said. But why do you want an official Cypress option for the job?

@keeema
Copy link
Author

keeema commented May 1, 2020

Because of the simplicity.

You have one, widely used, opinionated solution. The standard way.
With this, it is much easier to maintain just the tsconfig.json and version of typescript you have in dependencies.

With webpack preprocessor you have to maintain also
up-to-date ts-loader, webpack, webpack-preprocessor, webpack config.

You have to do this accross all projects, plugins etc.

I was enthusiastic when I was removing these things from my projects and dissapointed when I noticed I have to return them back...

@sainthkh
Copy link
Contributor

sainthkh commented May 1, 2020

Now I get it. One of the reason I added out-of-the-box typescript support is to remove that tediousness. Maybe I forgot how frustrating it was.

But adding global option to the project isn't the thing I can't decide alone as a collaborator. I need to discuss with other members first.

@keeema
Copy link
Author

keeema commented May 1, 2020

Thank you very much. I wait for the result with big hope.

@sainthkh sainthkh added stage: needs investigating Someone from Cypress needs to look at this stage: proposal 💡 No work has been done of this issue and removed stage: awaiting response Potential fix was proposed; awaiting response v4.4.0 🐛 Issue present since 4.4.0 stage: needs investigating Someone from Cypress needs to look at this labels May 1, 2020
@scottohara
Copy link

After reading through this issue, I'm a bit confused about whether Cypress uses a user-supplied tsconfig.json or not.

The updated Typescript docs mention configuring a tsconfig.json file, but it sounds like Cypress doesn't actually use the compilerOptions specified in that file when compiling/running the tests?

Is that correct? If so, the docs should perhaps make this a bit clearer.

The reason I'm interested is that my existing config for @cypress/webpack-preprocessor includes:

resolve: {
	...
	modules: [
		path.resolve(__dirname, "support"),
		...
	]
}

This allows test files in /cypress/integration/* to resolve import specifiers relative to the /cypress/support/* directory, e.g.

// cypress/support/utils.ts
export function foo() { ... };

// cypress/integration/test.ts
import { foo } from "utils";  // <- webpack resolves this to cypress/support/utils.ts

With built-in Typescript support, I was hoping to remove webpack-preprocessor and achieve the same with this in my cypress/tsconfig.json:

{
  "compilerOptions": {
    "baseUrl": ".",
	"paths": {
		"*": [
			"support/*"
		]
	},

Unfortunately with webpack-preprocessor removed, when Cypress runs my specs, I get errors like:

Error: Cannot find module '<thing in support>' from '<path to my test file>'

After reading this issue, I'm starting to think that the problem is the baseUrl and paths settings in tsconfig.json are ignored by Cypress?

@keeema
Copy link
Author

keeema commented May 24, 2020

It doesn't, unfortunately. I hope it will be added in next version.

@keeema
Copy link
Author

keeema commented Jun 23, 2020

Hi all, any update or plans on this issue?

@sainthkh
Copy link
Contributor

My current focus is #7716. Maybe I'll try to start the discussion after that. But don't get your hope high yet. Things can change.

@DarkMikey
Copy link

@scottohara Since nothing really changed here but I think more people will have this issue: Was there any way around this?

After reading this issue, I'm starting to think that the problem is the baseUrl and paths settings in tsconfig.json are ignored by Cypress?

@scottohara
Copy link

Hey @DarkMikey, in my case the only compiler options I needed Cypress to read from my tsconfig.json file was the baseUrl and paths properties; as I wanted to be able to use shorter import specifiers in my specs, e.g.

// cypress/integration/test.ts
import { foo } from "utils";  // <- I want this to resolve to cypress/support/utils.ts

The workaround for me (while awaiting Cypress support) was to simply switch to a relative import path specifier, e.g.

// cypress/integration/test.ts
import { foo } from "../support/utils";

If, like the OP, you need Cypress to recognise other compiler options such as strict etc. then I'm not aware of any other workarounds for that.

Hope this helps.

@DarkMikey
Copy link

Thank you very much. You're the first one I've found that has the exact same problem. Have you already opened an own issue (since this could be differently to the OPs problem)?

The thing is, if I import foo by import { foo } from "../support/utils"; and foo has an absolute import itself it fails recursively. I cannot change every import in the project to a relative import so I guess I have to keep the typescript-webpack-preprocessor 😣

Apparently this took me way too long to debug. Like @scottohara already suggested please mention something in the docs! In my desperation I've also opened a SO question yesterday.

@chrisbreiding
Copy link
Contributor

There are 2 different issues here:

  1. A user's tsconfig.json is not loaded by Cypress. This is covered by this issue and is being addressed by this PR, which will be released in v5.0 in the near future. @scottohara, @DarkMikey, please follow along with that issue/PR as it will fix any issues with baseUrl/paths config.

  2. Cypress only transpiles TypeScript and does not type-check it, so even if tsconfig.json is loaded, strict: true will not have any effect. This will still be the default functionality going forward. Whether or not we add an option to enable type-checking is a matter of debate. I'm going to discuss this with team members and we'll make a decision.

@nicholaschiang
Copy link

nicholaschiang commented Sep 2, 2020

@chrisbreiding it seems like even in v5.1 Cypress is still not properly loading the user's tsconfig.json. Taking a look at #7006, it seems like the baseUrl TSC compiler option is not being handled properly:

image

And that's still reproducible using the original repro, Cypress 5.1.0 and Typescript 4.0.2.

@nicholaschiang
Copy link

@DarkMikey Do you have a fix? Am I missing something obvious here?

@chrisbreiding
Copy link
Contributor

@nicholaschiang Path aliases were fixed for spec files, which are bundled for the browser, but still don't work for the plugins file, which is run in Node. I opened a new issue for this: #8555

@chrisbreiding
Copy link
Contributor

In regards to the original issue, Cypress now supports most compiler options for spec files, but will continue to only transpile files by default and not type check them for performance reasons, so setting "strict": true will have no effect.

We do not plan to add a global option to configure this, as it can be accomplished through the preprocessor API.

@keeema
Copy link
Author

keeema commented Sep 17, 2020

I still don't understand where is the problem in allowing it.
I am developer so I should decide whether I prefer faster build or safer build. Web-pack pre-processor is just another unnecessary configuration I have to maintain...

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

7 participants