-
Notifications
You must be signed in to change notification settings - Fork 73
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
Update RPC documentation #422
Conversation
Updates the outdated documentation for setting up RPC services by - Removing any notion of the old vscode json-rpc protocol. The neutral term RPC is used instead - Replacing the outdated loggingServer example with a current version of the taskServer - Removing sections of the documentation that are now obsolete because the underlying framework code has been reworked Note: he code snippets are based on eclipse-theia/theia#12581. So this PR is blocked until the referenced PR is approved and merged (the last section Part of eclipse-theia/theia#12368
src/docs/json_rpc.md
Outdated
``` | ||
The magic here is that this `ConnectionHandler` type is bound to a | ||
ContributionProvider. A central `MessagingContribution` picks up all registered connection handlers | ||
an when this contribution is initialized it t creates a websocket channel for all bound `ConnectionHandlers`. |
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.
...it creates as websocket...
like so (from messaging-module.ts): | ||
|
||
``` typescript | ||
constructor( @inject(ContributionProvider) @named(ConnectionHandler) protected readonly handlers: ContributionProvider<ConnectionHandler>) { |
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.
Not necessary?
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.
This snippet is outdated. The replacement in the current codebase would be the MessagingContribution
.
However, this classe hides most of the relevant logic behind abstractions. I don't think a snippet of the new API is really helpful in this high level introduction . It's potentially more confusing than it helps.
So here at the last line we're binding the ILoggerServer interface to a | ||
JsonRpc proxy. | ||
|
||
Note that his under the hood calls: |
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.
Again, do you consider this code snippet unneccessary?
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.
Readded with updated snippet.
- Rename `JsonRpcProxyFactory` and related types to `RpcProxyFactory` (Rpc*). The naming scheme was a remainder of the old vscode jsonr-rpc based protocol. By simply using the `Rpc` suffix the class names are less misleading and protocol agnostic. - Keep deprecated declarations of the old `JsonRpc*` namespace. The components are heavily used by adopters so we should maintain this deprecated symbols for a while to enable graceful migration without hard API breaks. Naturally this is open for discussion, but judging from the files I had to touch in the core framework alone I think it's definitely a good idea to give adopters a grace period. Complementary website PR: eclipse-theia/theia-website#422 Fixes #12581 Contributed on behalf of STMicroelectronics
- Rename `JsonRpcProxyFactory` and related types to `RpcProxyFactory` (Rpc*). The naming scheme was a remainder of the old vscode jsonr-rpc based protocol. By simply using the `Rpc` suffix the class names are less misleading and protocol agnostic. - Keep deprecated declarations of the old `JsonRpc*` namespace. The components are heavily used by adopters so we should maintain this deprecated symbols for a while to enable graceful migration without hard API breaks. Naturally this is open for discussion, but judging from the files I had to touch in the core framework alone I think it's definitely a good idea to give adopters a grace period. Complementary website PR: eclipse-theia/theia-website#422 Fixes #12581 Contributed on behalf of STMicroelectronics
- Rename `JsonRpcProxyFactory` and related types to `RpcProxyFactory` (Rpc*). The naming scheme was a remainder of the old vscode jsonr-rpc based protocol. By simply using the `Rpc` suffix the class names are less misleading and protocol agnostic. - Keep deprecated declarations of the old `JsonRpc*` namespace. The components are heavily used by adopters so we should maintain this deprecated symbols for a while to enable graceful migration without hard API breaks. Naturally this is open for discussion, but judging from the files I had to touch in the core framework alone I think it's definitely a good idea to give adopters a grace period. Complementary website PR: eclipse-theia/theia-website#422 Fixes #12581 Contributed on behalf of STMicroelectronics
- Rename `JsonRpcProxyFactory` and related types to `RpcProxyFactory` (Rpc*). The naming scheme was a remainder of the old vscode jsonr-rpc based protocol. By simply using the `Rpc` suffix the class names are less misleading and protocol agnostic. - Keep deprecated declarations of the old `JsonRpc*` namespace. The components are heavily used by adopters so we should maintain this deprecated symbols for a while to enable graceful migration without hard API breaks. Naturally this is open for discussion, but judging from the files I had to touch in the core framework alone I think it's definitely a good idea to give adopters a grace period. Complementary website PR: eclipse-theia/theia-website#422 Fixes #12581 Contributed on behalf of STMicroelectronics
- Rename `JsonRpcProxyFactory` and related types to `RpcProxyFactory` (Rpc*). The naming scheme was a remainder of the old vscode jsonr-rpc based protocol. By simply using the `Rpc` suffix the class names are less misleading and protocol agnostic. - Keep deprecated declarations of the old `JsonRpc*` namespace. The components are heavily used by adopters so we should maintain this deprecated symbols for a while to enable graceful migration without hard API breaks. Naturally this is open for discussion, but judging from the files I had to touch in the core framework alone I think it's definitely a good idea to give adopters a grace period. Complementary website PR: eclipse-theia/theia-website#422 Fixes #12581 Contributed on behalf of STMicroelectronics
- Rename `JsonRpcProxyFactory` and related types to `RpcProxyFactory` (Rpc*). The naming scheme was a remainder of the old vscode jsonr-rpc based protocol. By simply using the `Rpc` suffix the class names are less misleading and protocol agnostic. - Keep deprecated declarations of the old `JsonRpc*` namespace. The components are heavily used by adopters so we should maintain this deprecated symbols for a while to enable graceful migration without hard API breaks. Naturally this is open for discussion, but judging from the files I had to touch in the core framework alone I think it's definitely a good idea to give adopters a grace period. Complementary website PR: eclipse-theia/theia-website#422 Fixes #12581 Contributed on behalf of STMicroelectronics
Updates the outdated documentation for setting up RPC services by
Note: he code snippets are based on eclipse-theia/theia#12581. So this PR is blocked until the referenced PR is approved and merged
Part of eclipse-theia/theia#12368
Contributed on behalf of STMicroelectronics