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

Allow language specific handlers to use their own types for serialization #72230

Merged
merged 7 commits into from
Apr 9, 2024

Conversation

dibarbet
Copy link
Member

@dibarbet dibarbet commented Feb 22, 2024

This moves the deserialization and serialization support out of StreamJsonRpc, and directly into roslyn. Before we deserialize, we figure out what language a particular handler is for (based on the text document in the parameters). If none is found we use the default language handler.

This is done by manually looking at the JToken for the uri in the expected location (LSP requests are generally uniform in where they put the URI).

This will allow XAML to use different instances of LSP serialization types as compared to Roslyn.

request,
lspServices,
_logger,
cancellationToken);
}

protected virtual string GetLanguageForRequest<TRequest>(string methodName, TRequest request)
=> LanguageServerConstants.DefaultLanguageName;
//protected virtual string GetLanguageForRequest<TRequest>(string methodName, TRequest request)
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems to be no longer used anywhere

@dibarbet dibarbet force-pushed the fix_serialization branch 2 times, most recently from 042a611 to cd4619b Compare April 3, 2024 01:54
@dibarbet
Copy link
Member Author

dibarbet commented Apr 4, 2024

Originally I was planning on pushing the deserialization down deeper into the queue (instead of before we enqueue it). However, I encountered a couple issues

  1. In order to create the request context, the request needs to be deserialized. Unfortunately this has to happen serially so we cannot parallelize the deserialization like I was hoping.
  2. While possible to put it in the queue, it dramatically increases the complexity of this change (many of the clasp types need to be reworked as they're expecting the real types by the time a request gets enqueued).

Additionally, I am hopeful that in the future we may be able to provide a protocol types library that can handle all of the non-breaking JSON protocol changes without binary breaking changes (generally these occur because of union type additions). This is a requirement for Razor cohosting as well. If we're able to convert XAML to using those types as well, we would no longer need to support different type definitions for the same LSP method.

@dibarbet dibarbet changed the title [WIP] Allow language specific handlers to use their own types for serialization Allow language specific handlers to use their own types for serialization Apr 4, 2024
@dibarbet dibarbet marked this pull request as ready for review April 4, 2024 22:52
@dibarbet dibarbet requested a review from a team as a code owner April 4, 2024 22:52
Copy link
Contributor

@ToddGrun ToddGrun left a comment

Choose a reason for hiding this comment

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

:shipit:

@dibarbet
Copy link
Member Author

dibarbet commented Apr 8, 2024

Update - the PR val cloudbuild failed due to a warning triggering in the source file includes. Running a new one now, should have results soon.

@dibarbet dibarbet merged commit efdb565 into dotnet:main Apr 9, 2024
27 checks passed
@dibarbet dibarbet deleted the fix_serialization branch April 9, 2024 17:37
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Apr 9, 2024
@dibarbet dibarbet modified the milestones: Next, 17.11 P1 Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants