[Razor] Support platform agnostic language server & telemetry #6600
Conversation
| { | ||
| "id": "Razor", | ||
| "description": "Razor Language Server (Platform Agnostic)", | ||
| "url": "https://download.visualstudio.microsoft.com/download/pr/f8b5b74b-3df3-47cc-83b1-cd1d93d1771d/e456a753103b84b87bf5f000499ce3d7/razorlanguageserver-platformagnostic-7.0.0-preview.23513.5.zip", |
There was a problem hiding this comment.
This version doesn't exactly match the version above since 23516.2 wasn't published with a platform agnostic language server. This is temporary and should be resolved whenever Razor next updates its versions.
There was a problem hiding this comment.
as soon as dotnet/roslyn#70568 merges we can bump razor and roslyn, in this PR perhaps?
| return await installPackageJsonDependency('Razor', packageJSON, platformInfo); | ||
| if (!(await installPackageJsonDependency('Razor', packageJSON, platformInfo))) { | ||
| // Try downloading platform neutral package instead | ||
| const platformNeutral = new PlatformInformation('neutral', 'neutral'); |
There was a problem hiding this comment.
I don't think we should have this fallback. If the platform doesn't exist that we're packaging the vsix for, that is an error in setup authoring.
Fine with having fallbacks at runtime. But not in creating the vsix. Otherwise we might randomly get official platform specific vsix's that have a platform neutral razor server.
There was a problem hiding this comment.
That's fair - I updated the code to not fallback in this method
There was a problem hiding this comment.
So when is platformInfo undefined here? When someone does a build and pack on a machine architecture we don't officially support? Or are we going to start producing platform agnostic builds of the whole extension?
There was a problem hiding this comment.
I think eventually once we do start publishing our language agnostic servers to the marketplace, this method would be changed to return undefined instead of throwing:
vscode-csharp/src/shared/platform.ts
Line 147 in 1a8a3a9
I made the condition undefined based on a similar Roslyn condition:
vscode-csharp/tasks/offlinePackagingTasks.ts
Line 122 in 1a8a3a9
| return await installPackageJsonDependency('Razor', packageJSON, platformInfo); | ||
| if (!(await installPackageJsonDependency('Razor', packageJSON, platformInfo))) { | ||
| // Try downloading platform neutral package instead | ||
| const platformNeutral = new PlatformInformation('neutral', 'neutral'); |
There was a problem hiding this comment.
So when is platformInfo undefined here? When someone does a build and pack on a machine architecture we don't officially support? Or are we going to start producing platform agnostic builds of the whole extension?
|
According to @dibarbet we don't actually publish the platform agnostic extension in the marketplace today, so essentially this PR is for future proofing more than anything. Due to this I think this would be fine to merge and just update the versions in the next Razor update |
|
Sounds good, thanks for clarifying. Either @ryzngard will be doing an insertion today, or I'll do one on my Monday, so it won't be in this state for long anyway. |
Supports platform agnostic language server + telemetry for Razor in the case that the user isn't using an officially supported platform and architecture.