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(compiler-cli): do not throw fatal error if extended type check fails #53896

Closed
wants to merge 1 commit into from

Conversation

devversion
Copy link
Member

Currently when the extended type check fails due to an import reference that cannot be generated, the fatal diagnostic is not caught and not properly exposed as a ts.Diagnostic that can be gracefully handled. This is inconsistent to non-extended type checking diagnostics.

This is problematic because Angular CLI applications currently fail in obscure ways because:

  • the CLI does not expect getDiagnosticsForFile to actually throw runtime errors.
  • the CLI does not seem to properly print these errors given the parallel workers and build excection, and those errors are especially hard to debug because there is no stack for FatalDiagnosticError's.

Example: MyDir is not exported and the type check block cannot reference it.

CLI currently will fail with errors like:

> ng serve

⠼ Building...
Application bundle generation failed. [3.335 seconds]
✘ [ERROR] Cannot read properties of null (reading 'errors') [plugin angular-compiler]

    node_modules/@angular-devkit/build-angular/src/tools/esbuild/angular/compiler-plugin.js:257:24:
      257 │         if (diagnostics.errors?.length) {
          ╵                         ^

    at /usr/local/google/home/pgschwendtner/projects/test-next/node_modules/@angular-devkit/build-angular/src/tools/esbuild/angular/compiler-plugin.js:257:25
    at async /usr/local/google/home/pgschwendtner/projects/test-next/node_modules/esbuild/lib/main.js:1350:22
    at async Promise.all (index 0)
    at async requestCallbacks.on-start (/usr/local/google/home/pgschwendtner/projects/test-next/node_modules/esbuild/lib/main.js:1348:5)
    at async handleRequest (/usr/local/google/home/pgschwendtner/projects/test-next/node_modules/esbuild/lib/main.js:732:11)

cc. @clydin

@devversion devversion added action: review The PR is still awaiting reviews from at least one requested reviewer target: rc This PR is targeted for the next release-candidate labels Jan 12, 2024
@devversion devversion added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 12, 2024
Currently when the extended type check fails due to an import reference
that cannot be generated, the fatal diagnostic is not caught and
not properly exposed as a `ts.Diagnostic` that can be gracefully
handled. This is inconsistent to non-extended type checking diagnostics.

This is problematic because Angular CLI applications currently fail in
obscure ways because:

- the CLI does not expect `getDiagnosticsForFile` to actually throw
  runtime errors.
- the CLI does not seem to properly print these errors given the
  parallel workers and build excection, and those errors are
  especially hard to debug because there is no `stack` for
  `FatalDiagnosticError`'s.

Example: `MyDir` is not exported and the type check block cannot reference it.
@dylhunn
Copy link
Contributor

dylhunn commented Jan 16, 2024

This PR was merged into the repository by commit 760b1f3.

dylhunn pushed a commit that referenced this pull request Jan 16, 2024
…ils (#53896)

Currently when the extended type check fails due to an import reference
that cannot be generated, the fatal diagnostic is not caught and
not properly exposed as a `ts.Diagnostic` that can be gracefully
handled. This is inconsistent to non-extended type checking diagnostics.

This is problematic because Angular CLI applications currently fail in
obscure ways because:

- the CLI does not expect `getDiagnosticsForFile` to actually throw
  runtime errors.
- the CLI does not seem to properly print these errors given the
  parallel workers and build excection, and those errors are
  especially hard to debug because there is no `stack` for
  `FatalDiagnosticError`'s.

Example: `MyDir` is not exported and the type check block cannot reference it.

PR Close #53896
@dylhunn dylhunn closed this in 760b1f3 Jan 16, 2024
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…ils (angular#53896)

Currently when the extended type check fails due to an import reference
that cannot be generated, the fatal diagnostic is not caught and
not properly exposed as a `ts.Diagnostic` that can be gracefully
handled. This is inconsistent to non-extended type checking diagnostics.

This is problematic because Angular CLI applications currently fail in
obscure ways because:

- the CLI does not expect `getDiagnosticsForFile` to actually throw
  runtime errors.
- the CLI does not seem to properly print these errors given the
  parallel workers and build excection, and those errors are
  especially hard to debug because there is no `stack` for
  `FatalDiagnosticError`'s.

Example: `MyDir` is not exported and the type check block cannot reference it.

PR Close angular#53896
rlmestre pushed a commit to rlmestre/angular that referenced this pull request Jan 26, 2024
…ils (angular#53896)

Currently when the extended type check fails due to an import reference
that cannot be generated, the fatal diagnostic is not caught and
not properly exposed as a `ts.Diagnostic` that can be gracefully
handled. This is inconsistent to non-extended type checking diagnostics.

This is problematic because Angular CLI applications currently fail in
obscure ways because:

- the CLI does not expect `getDiagnosticsForFile` to actually throw
  runtime errors.
- the CLI does not seem to properly print these errors given the
  parallel workers and build excection, and those errors are
  especially hard to debug because there is no `stack` for
  `FatalDiagnosticError`'s.

Example: `MyDir` is not exported and the type check block cannot reference it.

PR Close angular#53896
danieljancar pushed a commit to danieljancar/angular that referenced this pull request Jan 26, 2024
…ils (angular#53896)

Currently when the extended type check fails due to an import reference
that cannot be generated, the fatal diagnostic is not caught and
not properly exposed as a `ts.Diagnostic` that can be gracefully
handled. This is inconsistent to non-extended type checking diagnostics.

This is problematic because Angular CLI applications currently fail in
obscure ways because:

- the CLI does not expect `getDiagnosticsForFile` to actually throw
  runtime errors.
- the CLI does not seem to properly print these errors given the
  parallel workers and build excection, and those errors are
  especially hard to debug because there is no `stack` for
  `FatalDiagnosticError`'s.

Example: `MyDir` is not exported and the type check block cannot reference it.

PR Close angular#53896
amilamen pushed a commit to amilamen/angular that referenced this pull request Jan 26, 2024
…ils (angular#53896)

Currently when the extended type check fails due to an import reference
that cannot be generated, the fatal diagnostic is not caught and
not properly exposed as a `ts.Diagnostic` that can be gracefully
handled. This is inconsistent to non-extended type checking diagnostics.

This is problematic because Angular CLI applications currently fail in
obscure ways because:

- the CLI does not expect `getDiagnosticsForFile` to actually throw
  runtime errors.
- the CLI does not seem to properly print these errors given the
  parallel workers and build excection, and those errors are
  especially hard to debug because there is no `stack` for
  `FatalDiagnosticError`'s.

Example: `MyDir` is not exported and the type check block cannot reference it.

PR Close angular#53896
nikvarma pushed a commit to nikvarma/angular that referenced this pull request Jan 31, 2024
…ils (angular#53896)

Currently when the extended type check fails due to an import reference
that cannot be generated, the fatal diagnostic is not caught and
not properly exposed as a `ts.Diagnostic` that can be gracefully
handled. This is inconsistent to non-extended type checking diagnostics.

This is problematic because Angular CLI applications currently fail in
obscure ways because:

- the CLI does not expect `getDiagnosticsForFile` to actually throw
  runtime errors.
- the CLI does not seem to properly print these errors given the
  parallel workers and build excection, and those errors are
  especially hard to debug because there is no `stack` for
  `FatalDiagnosticError`'s.

Example: `MyDir` is not exported and the type check block cannot reference it.

PR Close angular#53896
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker target: rc This PR is targeted for the next release-candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants