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): avoid duplicate diagnostics about unknown pipes #39517

Closed
wants to merge 1 commit into from

Conversation

alxhub
Copy link
Member

@alxhub alxhub commented Oct 30, 2020

TCB generation occasionally transforms binding expressions twice, which can
result in a BindingPipe operation being resolve()'d multiple times. When
the pipe does not exist, this caused multiple OOB diagnostics to be recorded
about the missing pipe.

This commit fixes the problem by making the OOB recorder track which pipe
expressions have had diagnostics produced already, and only producing them
once per expression.

TCB generation occasionally transforms binding expressions twice, which can
result in a `BindingPipe` operation being `resolve()`'d multiple times. When
the pipe does not exist, this caused multiple OOB diagnostics to be recorded
about the missing pipe.

This commit fixes the problem by making the OOB recorder track which pipe
expressions have had diagnostics produced already, and only producing them
once per expression.
@google-cla google-cla bot added the cla: yes label Oct 30, 2020
@alxhub alxhub requested a review from kyliau October 30, 2020 22:26
@alxhub alxhub added area: compiler Issues related to `ngc`, Angular's template compiler area: language-service Issues related to Angular's VS Code language service labels Oct 30, 2020
@ngbot ngbot bot modified the milestone: needsTriage Oct 30, 2020
@alxhub alxhub added the target: patch This PR is targeted for the next patch release label Oct 30, 2020
@ayazhafiz
Copy link
Member

I think it may be better to deduplicate diagnostics directly in the OOB recorder in case similar issues come up in the future. What do you think?

@alxhub
Copy link
Member Author

alxhub commented Nov 2, 2020

I think it may be better to deduplicate diagnostics directly in the OOB recorder in case similar issues come up in the future. What do you think?

Hmm, I don't know what you're suggesting should be done differently. This PR deduplicates the missing pipe diagnostic in the OOB recorder.

@kyliau
Copy link
Contributor

kyliau commented Nov 3, 2020

@ayazhafiz ayazhafiz added this to PRs In Review in Ivy Language Service Nov 3, 2020
@alxhub alxhub added the action: merge The PR is ready for merge by the caretaker label Nov 6, 2020
@mhevery mhevery closed this in cf88ea0 Nov 6, 2020
mhevery pushed a commit that referenced this pull request Nov 6, 2020
…39517)

TCB generation occasionally transforms binding expressions twice, which can
result in a `BindingPipe` operation being `resolve()`'d multiple times. When
the pipe does not exist, this caused multiple OOB diagnostics to be recorded
about the missing pipe.

This commit fixes the problem by making the OOB recorder track which pipe
expressions have had diagnostics produced already, and only producing them
once per expression.

PR Close #39517
mhevery pushed a commit that referenced this pull request Nov 6, 2020
…39517)

TCB generation occasionally transforms binding expressions twice, which can
result in a `BindingPipe` operation being `resolve()`'d multiple times. When
the pipe does not exist, this caused multiple OOB diagnostics to be recorded
about the missing pipe.

This commit fixes the problem by making the OOB recorder track which pipe
expressions have had diagnostics produced already, and only producing them
once per expression.

PR Close #39517
@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 Dec 7, 2020
@alxhub alxhub moved this from PRs In Review to Done in Ivy Language Service Dec 8, 2020
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 area: language-service Issues related to Angular's VS Code language service cla: yes target: patch This PR is targeted for the next patch release
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants