Skip to content

Commit

Permalink
fix: force noEmitOnError: false (#338)
Browse files Browse the repository at this point in the history
- `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
  • Loading branch information
agilgur5 committed Jun 1, 2022
1 parent b08f272 commit b44b069
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 1 deletion.
3 changes: 2 additions & 1 deletion README.md
Expand Up @@ -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*)
Expand Down
1 change: 1 addition & 0 deletions __tests__/get-options-overrides.spec.ts
Expand Up @@ -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`,
Expand Down
1 change: 1 addition & 0 deletions src/get-options-overrides.ts
Expand Up @@ -13,6 +13,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,
Expand Down

0 comments on commit b44b069

Please sign in to comment.