stryker-js: Discussion: TypeScript compile errors with mutation switching

See #1514 .

In a mutation switching world, we will create (typescript/flow) type errors. This is by design; it would be almost impossible to prevent it (we would need a team and community at least as large as https://github.com/Microsoft/TypeScript).

I don’t think flow will be an issue because flow code is transpiled without type checking (using a babel plugin). However, TypeScript is a different beast all together…

There are a couple of approaches I’ve tried:

Attempt 1: Transpile only

Running the typescript transpiler with configuration similar to ts-node’s --transpile-only.

However, after a little investigation, I learned that transpileOnly only works with --isolatedModules. This means that only a subset of typescript will be supported (although a best practice IMHO). We can’t rely on this for all our users.

The same goes for skipping type checking by using babel; it won’t support all use cases.

Attempt 2: Skip Typechecking

With a little fudging in the TypeScript compiler api you can get a long way. It would allow us to basically transpile all typescript projects while skipping type checks. See https://github.com/microsoft/TypeScript/issues/29651#issuecomment-616113744 for an example.

However, this approach falls short when you consider more typescript use cases. Calling the TypeScript compiler directly is just one of the ways typescript compilation might take place. A non-extensive list of ways to compile typescript:

  • ts-node
  • angular-cli
  • vue-cli
  • react-scripts
  • karma-typescript
  • webpack with ts-loader
  • webpack with awseome-ts-loader
  • jest-ts
  • rollup with typescript plugin
  • Probably 100s more of tools/plugins

Solution: // @ts-nocheck

The solution I ended up with is to use the // @ts-nocheck directive on top of each file (js or ts, since js files can also result in type errors). It works from TypeScript 3.7 onward and will work with all use cases (as long as you’re using a fairly recent TypeScript version).

Note that errors can also be produced by test files, even though they’re not mutated. Mutating production code can change the return type, for example function foo() { return 'bar'; } is mutated to function foo() { activeMutant === 1? {} : return 'bar'; }, which changes it’s return type from string to string | undefined, which probably results in a type error somewhere in your test files

Problem 1: Still type checking…

However, this presented me with another issue. Even with @ts-nocheck, a file can still provide type errors, this is because other directives can result in errors, for example @ts-expect-error, but @ts-check has the same result.

That’s why I decided to remove all comments from JS and TS files. We’re using the strip-comments package for that, which uses regular expressions to identify the comments.

You can configure this with 2 settings:

  "sandbox": {
    "stripComments": "**/*+(.js|.ts|.cjs|.mjs)?(x)",
    "fileHeaders": {
      "**/*+(.js|.ts|.cjs|.mjs)?(x)": "/* eslint-disable */\n// @ts-nocheck\n"
    }
  }

(see readme for specifics)

Problem 2: significant comments

I assumed stripping comments wouldn’t cause a problem, because comments should be comments. However, this is JavaScript we’re talking about. This is a list of comments that are significant and we should actually keep if possible:

Personally, I feel like removing all comments from all files is a shotgun approach. Simply removing only @ts-xxx comments would be better, but how would we do that? I don’t want to be parsing all input files (seriously, it is already a pain to parse only the files to be mutated) and strip-comments doesn’t support comment filtering.

Problem 3: strip-comments has known issues

We’ve recently found an issue with the strip-comments package where it leaves a file in an invalid state. If this would happen to someone than the only thing he/she can do is to disable the removal of comments altogether (with "sandbox.stripComments": false) or only for that file specifically.

Problem 4: incompatible scenario with jest.

Even though users can customize to fit their use case with stripComments and fileHeaders, we still don’t support all scenarios. For example, running @stryker-mutator/jest-runner together with the jest-ts jest plugin might result in a situation where you don’t want to strip comments (because of @jest-environment), but need to strip comments because you’re using @ts-expect-error somewhere in your tests.

Any input would be very helpful 😇

@hugo-vrijswijk @brodybits @Lakitna

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Reactions: 1
  • Comments: 17 (11 by maintainers)

Commits related to this issue

Most upvoted comments

@nicojs right now:

Almost correct @Lakitna 😃

This is the process:

https://github.com/stryker-mutator/stryker/blob/9c846bf618af487815074e3b49d674cf1786cdcd/packages/core/src/process/2-MutantInstrumenterExecutor.ts#L30-L38

  1. Instrument the code (works for JS, TS files as well as HTML/Vue files (script tags are instrumented)).
  2. “Preprocess” the files. Disable type checking is one of the preprocessors

We might open up the “preprocess” API in the future, but for now, we’re keeping it private. Public API’s are a hassle to maintain.

To disable type checking, we will need to parse the file, but performance impact is kept to a minimum because it will only parse when "@ts-" can be found anywhere in the file 😎

Follow the work in progress here: #2446

I’ve tested it on stryker core and, even without the regex I proposed, I don’t see a negative performance impact. And more importantly, it works like a charm! I’m really happy with this result. 😀

I think that makes the most sense in general

Glad you agree. 🙏 And thanks for the help figuring it out.

How about this: **/*.{js,ts,html,vue}?(x)

Sure. Maybe it’s even better to use **/*.{js,jsx,ts,tsx,html,vue}, since htmlx and vuex don’t make sense and listing them all might be even more clear.