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

fix: force noEmitOnError: false #338

Merged
merged 1 commit into from
Jun 1, 2022

Conversation

agilgur5
Copy link
Collaborator

@agilgur5 agilgur5 commented Jun 1, 2022

Summary

Force noEmitOnError: false for the same reason we force noEmit: false

Details

  • 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

  • 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

Can see more details in my root cause analysis in #254 (comment)

Related feature request: #168

- `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
@agilgur5 agilgur5 added the kind: bug Something isn't working properly label Jun 1, 2022
@ezolenko ezolenko merged commit b44b069 into ezolenko:master Jun 1, 2022
@agilgur5 agilgur5 added the topic: TS Compiler API Docs Related to the severely lacking TS Compiler API Docs label Jul 5, 2022
@agilgur5 agilgur5 deleted the fix-force-noEmitOnError-false branch July 2, 2023 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Something isn't working properly topic: TS Compiler API Docs Related to the severely lacking TS Compiler API Docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vague failed to transpile error -- noEmitOnError: true breaks rpt2
2 participants