Skip to content

refactor: use built-in WorkDoneProgressReport to report NGCC progress #1614

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

Merged
merged 1 commit into from
Mar 1, 2022

Conversation

ivanwonder
Copy link
Contributor

@ivanwonder ivanwonder commented Feb 27, 2022

Now the Angular extension implements the WorkDoneProgressReport
by itself on the client-side to support the multi-report progress
with different tsconfig.json, to show an error message, to
dispose of the report progress, but it's already a built-in feature
that can be used on the service side.

On the service side, the built-in WorkDoneProgressReport will
attach a unique token to the different report progress and clean
the progress when the client is closing.

On the service side, If an error report when running NGCC, the LSP
also supports showing an error message in VSCode, but the ability to
open the output channel is not supported(now the LSP only supports
to open the document URI). This PR will only need to provide it.

So the custom WorkDoneProgressReport can be removed on the client side.

@atscott
Copy link
Collaborator

atscott commented Feb 28, 2022

This is actually a really nice cleanup. Could you provide some more details in the commit message about how this accomplishes the same thing as before, but with much less code? That is, drop in some of the context from #1607 (comment) and other comments in that thread.

Now the Angular extension implements the `WorkDoneProgressReport`
by itself on the client-side to support the multi-report progress
with different `tsconfig.json`, to show an error message, to
dispose of the report progress, but it's already a built-in feature
that can be used on the service side.

On the service side, the built-in `WorkDoneProgressReport` will
attach a unique token to the different report progress and clean
the progress when the client is closing.

On the service side, If an error report when running NGCC, the LSP
also supports showing an error message in VSCode, but the ability to
open the output channel is not supported(now the LSP only supports
to open the document URI). This PR will only need to provide it.

So the custom `WorkDoneProgressReport` can be removed on the client side.
@ivanwonder ivanwonder force-pushed the refactor-progress-report branch from 71fe4a5 to 1a6005c Compare March 1, 2022 12:48
@ivanwonder ivanwonder marked this pull request as ready for review March 1, 2022 12:49
@ivanwonder ivanwonder requested a review from atscott March 1, 2022 12:49
@ivanwonder
Copy link
Contributor Author

After the tip Initializing Angular language features disappeared, there is a delay to show the tip Running ngcc for ./tsconfig.json, and I think it's acceptable.

Copy link
Collaborator

@atscott atscott left a comment

Choose a reason for hiding this comment

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

🥇

@atscott atscott added target: patch This PR is targeted for the next patch release action: merge Ready to merge labels Mar 1, 2022
@atscott atscott linked an issue Mar 1, 2022 that may be closed by this pull request
2 tasks
@atscott atscott merged commit 5157872 into angular:master Mar 1, 2022
atscott pushed a commit that referenced this pull request Mar 1, 2022
…ss (#1614)

Now the Angular extension implements the `WorkDoneProgressReport`
by itself on the client-side to support the multi-report progress
with different `tsconfig.json`, to show an error message, to
dispose of the report progress, but it's already a built-in feature
that can be used on the service side.

On the service side, the built-in `WorkDoneProgressReport` will
attach a unique token to the different report progress and clean
the progress when the client is closing.

On the service side, If an error report when running NGCC, the LSP
also supports showing an error message in VSCode, but the ability to
open the output channel is not supported(now the LSP only supports
to open the document URI). This PR will only need to provide it.

So the custom `WorkDoneProgressReport` can be removed on the client side.

(cherry picked from commit 5157872)
@ivanwonder ivanwonder deleted the refactor-progress-report branch March 1, 2022 23:31
@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 Apr 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge Ready to merge cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Progress notifications lack kind field
2 participants