Skip to content

fix(compiler-cli): do not throw fatal error if extended type check fails#53896

Closed
devversion wants to merge 1 commit intoangular:mainfrom
devversion:fix-cli-breaking
Closed

fix(compiler-cli): do not throw fatal error if extended type check fails#53896
devversion wants to merge 1 commit intoangular:mainfrom
devversion:fix-cli-breaking

Conversation

@devversion
Copy link
Member

@devversion devversion commented 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.

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 requested a review from crisbeto January 12, 2024 12:58
@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
@devversion
fix(compiler-cli): do not throw fatal error if extended type check fails
edd1016 
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
@devversion @dylhunn
fix(compiler-cli): do not throw fatal error if extended type check fa…
76ceeba 
…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
@devversion @ChellappanRajan
fix(compiler-cli): do not throw fatal error if extended type check fa…
16947de 
…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
@devversion @rlmestre
fix(compiler-cli): do not throw fatal error if extended type check fa…
9f669ae 
…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
@devversion @amilamen
fix(compiler-cli): do not throw fatal error if extended type check fa…
8bbe820 
…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
@devversion @nikvarma
fix(compiler-cli): do not throw fatal error if extended type check fa…
2ee7ab8 
…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

angular-automatic-lock-bot bot commented Feb 16, 2024

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.

Reviewers

@crisbeto crisbeto crisbeto approved these changes

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@devversion @dylhunn @crisbeto