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

Make the @ts-nocheck header configurable #2357

Closed
nicojs opened this issue Aug 6, 2020 · 4 comments · Fixed by #2363
Closed

Make the @ts-nocheck header configurable #2357

nicojs opened this issue Aug 6, 2020 · 4 comments · Fixed by #2363
Labels
🚀 Feature request New feature request
Milestone

Comments

@nicojs
Copy link
Member

nicojs commented Aug 6, 2020

Is your feature request related to a problem? Please describe.

In a mutation switching world, Stryker will add the // @ts-nocheck header to each .js, .ts, .tsx and .jsx file in the sandbox. This allows for the project to run even though mutators created type errors (users can opt-in to use the @stryker-mutator/typescript-checker to filter out invalid mutants). However, adding this header can have undesired side effects.

For example, the integration tests of @stryker-mutator/typescript-checker itself rely on files located in the testResources directory. They should not be prefixed, otherwise the tests will fail (because an error is actually expected, that's the goal of the test).

Describe the solution you'd like

I think the only way around these edge cases is to make adding this header configurable. I was thinking something like this:

{
  "sandbox": {
    "tsNoCheck": "+(src|test)/**/*.ts" // can also be a boolean
  }
}

The following options are allowed:

  • true would prefix each file (default).
  • false would result in completely disabling.
  • 'src/**/*.ts' only prefix files that matches this glob expression.

Describe alternatives you've considered

We can keep hard coding it, but disable it for testResources, but I don't like this magic default and it wouldn't work for all projects.

The configuration option name candidates:

  • sandbox.tsNoCheck
  • sandbox['@ts-nocheck']
  • sandbox.disableTypeChecking

I would like to make it a property of sandbox, in order to later add a mode property there as well (see #2163 ).

@simondel what do you think? It does add (additional) complexity, but the default would be fine for 99.9% of use cases anyhow IMHO, so it is acceptable?

@nicojs nicojs added the 🚀 Feature request New feature request label Aug 6, 2020
@nicojs nicojs added this to the 4.0 milestone Aug 6, 2020
@nicojs
Copy link
Member Author

nicojs commented Aug 7, 2020

Just discussed this with @hugo-vrijswijk . We're now thinking to make it a bit more generic. Also allowing this to work for other headers, like /* eslint-disable */.

{
  "preprocessHeaders": {
     "**/*+(.js|.ts)?(x)": "/* eslint-disable */\n// @ts-nocheck\n"
   }
}

@simondel
Copy link
Member

simondel commented Aug 7, 2020

Sounds good!

@nicojs
Copy link
Member Author

nicojs commented Aug 7, 2020

After careful consideration, we're going with sandboxFileHeaders 🤷‍♂️

@nicojs
Copy link
Member Author

nicojs commented Aug 16, 2020

And after more careful considereration, I've changed it to sandbox: { fileHeaders: "**/*+(.js|.ts)?(x)": "/* eslint-disable */\n// @ts-nocheck\n" } } 🤷‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 Feature request New feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants