From 060459124cf6a4ddc06683483a1a969def99af38 Mon Sep 17 00:00:00 2001 From: Anton Gilgur Date: Wed, 1 Jun 2022 14:39:07 -0400 Subject: [PATCH] fix: force `noEmitOnError: false` - `noEmitOnError: true` acts like `noEmit: true` when there is an error - this is problematic because it will then cause _all_ files to have `emitSkipped` set to `true`, which this plugin interprets as a fatal error - meaning it will treat the first file it finds as having a fatal error and then abort, but possibly without outputting any diagnostics what-so-ever as the file with the error in it may not yet have run through the `transform` hook - i.e. an initial file that imports an erroring file at some point in its import chain will cause rpt2 to abort, even if that initial file _itself_ has no type-check/diagnostic issues - bc TS does whole-program analysis after all - this has only been reported as an issue once so far, probably because it defaults to `false` in TS and, as such, is rarely used: https://www.typescriptlang.org/tsconfig#noEmitOnError - we usually have the opposite issue, people trying to set it to `false` (i.e. the default) because they don't realize the `abortOnError` option exists - add `noEmitOnError: false` to the forced options list and tests too - add it to the docs on what tsconfig options are forced - and add a reference to the issue like the existing options - also reference `abortOnError` since they're commonly associated with each other and that plugin option is often missed (per above) - briefly explain that `noEmit` and `noEmitOnError` are `false` because Rollup controls emit settings in the context of this plugin, instead of `tsc` etc - should probably watch out for when new emit settings are added to TS, as we may want to force most with the same reasoning --- README.md | 3 ++- __tests__/get-options-overrides.spec.ts | 1 + src/get-options-overrides.ts | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index eed246e4..2aeded84 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,8 @@ The plugin inherits all compiler options and file lists from your `tsconfig.json * `noEmitHelpers`: false * `importHelpers`: true * `noResolve`: false -* `noEmit`: false +* `noEmit`: false (Rollup controls emit) +* `noEmitOnError`: false (Rollup controls emit. See [#254](https://github.com/ezolenko/rollup-plugin-typescript2/issues/254) and the `abortOnError` plugin option below) * `inlineSourceMap`: false (see [#71](https://github.com/ezolenko/rollup-plugin-typescript2/issues/71)) * `outDir`: `./placeholder` in cache root, see [#83](https://github.com/ezolenko/rollup-plugin-typescript2/issues/83) and [Microsoft/TypeScript#24715](https://github.com/Microsoft/TypeScript/issues/24715) * `declarationDir`: Rollup's `output.file` or `output.dir` (*only if `useTsconfigDeclarationDir` is false in the plugin options*) diff --git a/__tests__/get-options-overrides.spec.ts b/__tests__/get-options-overrides.spec.ts index 4960d904..ec2c6725 100644 --- a/__tests__/get-options-overrides.spec.ts +++ b/__tests__/get-options-overrides.spec.ts @@ -49,6 +49,7 @@ const forcedOptions: ts.CompilerOptions = { inlineSourceMap: false, moduleResolution: ts.ModuleResolutionKind.NodeJs, noEmit: false, + noEmitOnError: false, noEmitHelpers: false, noResolve: false, outDir: `${cacheDir}/placeholder`, diff --git a/src/get-options-overrides.ts b/src/get-options-overrides.ts index 86617617..693d49a2 100644 --- a/src/get-options-overrides.ts +++ b/src/get-options-overrides.ts @@ -12,6 +12,7 @@ export function getOptionsOverrides({ useTsconfigDeclarationDir, cacheRoot }: IO importHelpers: true, noResolve: false, noEmit: false, + noEmitOnError: false, inlineSourceMap: false, outDir: normalize(`${cacheRoot}/placeholder`), // need an outdir that is different from source or tsconfig parsing trips up. https://github.com/Microsoft/TypeScript/issues/24715 moduleResolution: tsModule.ModuleResolutionKind.NodeJs,