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): use @ts-ignore. #47636

Closed
wants to merge 1 commit into from
Closed

Conversation

mprobst
Copy link
Contributor

@mprobst mprobst commented Oct 5, 2022

The previous commit 2e1ddde used @ts-expect-error to suppress the current error, with the intent of being informed once that's no longer an error, ie. when we updated to an upstream TS version that includes this change.

However this unfortunately means the change is incompatible with the fixed version, which prevents it from working with an updated TS version in google3.

This change reverts back to the original @ts-ignore which is forwards and backwards compatible, avoiding that problem (but unfortunately losing the benefit of being notified once fixed).

This is a follow up to PR #47585.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

  • Bugfix

What is the current behavior?

See above.

Does this PR introduce a breaking change?

  • Yes
  • No

The previous commit 2e1ddde used `@ts-expect-error` to suppress the
current error, with the intent of being informed once that's no longer
an error, ie. when we updated to an upstream TS version that includes
this change.

However this unfortunately means the change is incompatible with the
fixed version, which prevents it from working with an updated TS version
in google3.

This change reverts back to the original `@ts-ignore` which is forwards
and backwards compatible, avoiding that problem (but unfortunately
losing the benefit of being notified once fixed).
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release action: presubmit The PR is in need of a google3 presubmit area: compiler Issues related to `ngc`, Angular's template compiler labels Oct 5, 2022
@ngbot ngbot bot added this to the Backlog milestone Oct 5, 2022
@ngbot
Copy link

ngbot bot commented Oct 5, 2022

I see that you just added the action: merge label, but the following checks are still failing:
    failure status "google3" is failing
    pending status "ci/angular: size" is pending

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken main, please try rebasing to main and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@AndrewKushnir AndrewKushnir added merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: presubmit The PR is in need of a google3 presubmit labels Oct 5, 2022
@AndrewKushnir
Copy link
Contributor

Merge-assistance: the size test failure is unrelated and likely a flake. This PR is ready for merge.

@jessicajaniuk
Copy link
Contributor

This PR was merged into the repository by commit 19ad498.

@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 Nov 5, 2022
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 area: compiler Issues related to `ngc`, Angular's template compiler merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants