-
Notifications
You must be signed in to change notification settings - Fork 22
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
Use vscode-messenger library for vscode messages #299
base: master
Are you sure you want to change the base?
Conversation
5d92b47
to
c0dfad9
Compare
A review is required for npm/npmjs/-/vscode-messenger-webview/0.5.1. A review is required for npm/npmjs/-/vscode-messenger-common/0.5.1. A review is required for npm/npmjs/-/vscode-messenger/0.5.1. |
c0dfad9
to
a597d44
Compare
FYI @ngondalia, @GregSavin regarding non-backwards compatible change and future follow-ups. |
Fixes eclipse-cdt-cloud#78 Non-backwards compatible change for downstream project when registering callbacks for webview messages. The key for the command key is changed from `message.command` to `method.method`. Signed-off-by: Bernd Hufmann <[email protected]>
a597d44
to
a04fdf0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this PR introduces a breaking change for consumers of the API, we can consider making this more "discoverable" by mentioning it in the commit message title. e.g. "[breaking] Use vscode-messenger library for vscode messages". This way the information would be mentioned in the auto-generated release notes.
We can also consider if we want to reflect breakage in the next version number we publish, that will contain this change. Since we are still < v1.0.0, our only lever would be to go to v1.0.0. (technically I think <v1.0.0, as we have currently, there is no formal promise of non-breakage)
What it does
Use vscode-messenger library for communication of extension and webviews.
The extension returns also a Diagnostic object (appended to the vscode-trace-extension's
ExternalAPI
so that message exchanges can be logged using the VS Code Messenger Developer Tool. This is purely for trouble shooting purposes.Fixes #78.
Note: There is a non-backward compatible change for third-party extension using interfacing the
vscode-trace-extension
with this change. The messenger library is usingmethod
instead ofcommand
for message keys. Downstream projects will need to update their callback methods (seemessage.method
below).How to test
Run and use the extension. No change in behavior is expected.
Follow-ups
Use vscode-messenger capabilities to send messages directly
webview
<->webview
/set of webviews
instead of going through the extension host which forwards them is using node signals as described here. This would simplify webview synchronization. However, the implementation for persisting of webview states (e.g. selected experiment) need to be changed. Not using Node signals would affect the external API of the vscode-trace-extension, regardingonSignalManagerSignal
andonSignalManagerSignal
as described here, and this has to be addressed when implementing it.We can also use message types
Request
andResponse
instead ofNotification
where applicable.We also have the possibility to introduce typing of the message data.
Review checklist