-
Notifications
You must be signed in to change notification settings - Fork 250
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
Allow a custom build command, instead of the transpiler plugin #2276
Comments
Discussed in-person with @hugo-vrijswijk and @simondel. These are the conclusions For issue 1: add a header to each js-like file which disables all linters/checkers. For example: // @ts-nocheck
/* eslint-disable */ We'll probably make it non-configurable and not-optional for now. For issue 2: |
Add an ignore header to each js (or friend) file in the sandbox. This should allow files to be transpiled no matter the contents. For more info, see #2276 The header now consists of: ```js /* eslint-disable */ // @ts-nocheck ``` But we can later decide to include more ignore rules.
Add an ignore header to each js (or friend) file in the sandbox. This should allow files to be transpiled no matter the contents. For more info, see #2276 The header now consists of: ```js /* eslint-disable */ // @ts-nocheck ``` But we can later decide to include more ignore rules.
Add a new `tsconfigFile` option (reused from the old `@stryker-mutator/typescript` package). Use this new option in the `@stryker-mutator/typescript-checker` instead of a specific typescript-checker specific one From stryker core, use this setting to rewrite `tsconfig.json` file's `references` and `extends` when they refer config files that fall outside of the sandbox. For example: ```json { "extends": "../../tsconfig.settings.json", "references": { "path": "../model" } } ``` becomes: ```json { "extends": "../../../../tsconfig.settings.json", "references": { "path": "../../../model" } } ``` The implementation relies on typescript being available, but will only import it when it found a tsconfig.json file. Once a tsconfig file is found, referenced files within the sandbox also get rewritten. Closes #2276
This issue is a continuation of the discussion started in #1514 (comment)
Transpiling code in a mutation switching world...
Example project
Before mutation switching, a user would configure the transpiler plugin (for example
transpilers: ["typescript"]
). This would result in the code being transpiled in-place and in-memory.Results were written to the sandbox:
Now, with mutation switching, we want to let users configure a custom build command. This way we don't need to support all different kinds of transpilers.
Example:
(*foo.ts is instrumented here)
The build command configured will run inside the sandbox-1234. The advantage here is a massive decrease of maintainance for the Stryker team. We no longer need to support all different kinds of transpilers (we're currently supporting webpack, typescript and babel)
However, 2 challenges remain with this approach:
foo.ts
will have type errors, and compiling it with thetsc
would not result in js output.tsconfig.json
's might have references outside the sandbox and would not compile. For example"extends": "../../tsconfig.settings.json"
.Dealing with (typescript, or eslint) errors
For the first challenge, there is a solution. This documented in microsoft/TypeScript#29651 (comment).
Basically we would have to add
// @ts-nocheck
to each and every ts/js file (also js because of--checkJS
mode). However, I'm hesitant to always add it.This might not be expected for end-users.
Note: even without a build command configured, we might need to add
// @ts-nocheck
. For example, thekarma-webpack
plugin won't work with type errors.Dealing with references to config files
For the second issue, the solution would be to change the contents of said tsconfig file.
"../../tsconfig.settings.json"
could be transformed to be"../../../../tsconfig.settings.json
. However, there are a few things to consider:.json
file is a typescript config file, other than educated guesses. People will need to configure one of moretsconfig
files, so we know which one to transform.ts.parseConfigFileTextToJson
). Seeing as projects with typescript will have typescript already installed, I think we can safelyrequire
typescript without needing a (peer)dependency on from core.With the current planned
@stryker-mutator/typescript-checker
plugin users already configure thetsconfig.json
file intypescriptChecker.tsconfigFile
.Note: this problem can also occur for babel. If you add a
babel.config.js
file, where yourequire
from the root for example.We could also opt to not transform config files, and instead implement #2163 sooner. I don't like that personally.
An alternative might be to let users do this fixup of config files themselves. For example, by splitting the instrumenting process from the mutation testing process. So this scenario would look like:
npx stryker instrument
.stryker-tmp/sandbox-123
directory and manually changes the config filesnpx stryker continue
.I don't like this approach either, personally.
We could of course also decide to keep the current
transpiler
plugin and accept the maintenance that comes along with it.@hugo-vrijswijk @simondel @kmdrGroch I'm curious to hear your thoughts on the matter.
The text was updated successfully, but these errors were encountered: