-
Notifications
You must be signed in to change notification settings - Fork 719
Adding changes to support use monovsdbg to debug wasm apps. #7220
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
base: main
Are you sure you want to change the base?
Conversation
src/shared/utils.ts
Outdated
@@ -43,6 +43,14 @@ export function isWebProject(projectPath: string): boolean { | |||
return projectFileText.toLowerCase().indexOf('sdk="microsoft.net.sdk.web"') >= 0; | |||
} | |||
|
|||
export function isWebAssemblyProject(projectPath: string): boolean { | |||
const projectFileText = fs.readFileSync(projectPath, 'utf8'); |
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.
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.
done
…_ASSEMBLIES to enable hotreload.
…/vscode-csharp into dev/thays/support_wasm_monovsdbg
@gregg-miskelly can you please review again? |
@gregg-miskelly I think I'm done again. If you can review it will be great. |
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.
Otherwise LGTM
Did you want to update a case for the updatePackageDependencies script?
|
I'm using npm run updatePackageDependencies to update the webassembly package but I didn't need to change this file. |
@thaystg Is this still a change we want to consider? |
Correct, we will work on it yet, but I will mark it as a draft for now. |
Can I get a review again here? |
@@ -707,6 +707,23 @@ | |||
"binaries": [ | |||
"./rzls" | |||
] | |||
}, | |||
{ | |||
"id": "VSWebAssemblyBridge", |
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.
Should this be added here so you can use the update dependencies script?
} | ||
}); | ||
assetsPath = assetsPath.slice(0, -1); | ||
return [assetsPath, this.executableProjects[0].outputPath]; |
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.
return [assetsPath, this.executableProjects[0].outputPath]; | ||
} | ||
|
||
public isDotNet9OrNewer(): boolean { |
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.
@@ -35,12 +35,15 @@ export function findNetStandardTargetFramework(tfmShortNames: string[]): string | |||
return tfmShortNames.find((tf) => tf.startsWith('netstandard')); | |||
} | |||
|
|||
export function isWebProject(projectPath: string): boolean { | |||
const projectFileText = fs.readFileSync(projectPath, 'utf8'); | |||
export function isWebProject(projectPath: string): [boolean, boolean] { |
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.
We now have two ways to detect a Blazor project. Should a project that is a Blazor project always have both? Or is one of them just going to return true for a subset of blazor projects?
Adding changes to support debug wasm apps using monovsdbg.
Tested scenarios with the new setting enabled and this PR:
With Assets and C# DevKit Not Installed targetting .NET 8 (Blazor Wasm) - DISABLED
With Assets and C# DevKit Not Installed targetting .NET 8 (Blazor Server Wasm) - DISABLED
With Assets and C# DevKit Not Installed targetting .NET 9 (Blazor Wasm) - ENABLED
With Assets and C# DevKit Not Installed targetting .NET 9 (Blazor Server Wasm) - ENABLED
With Assets and C# DevKit Installed targetting .NET 8 (Blazor Wasm) - DISABLED
With Assets and C# DevKit Installed targetting .NET 8 (Blazor Server Wasm) - DISABLED
With Assets and C# DevKit Installed targetting .NET 9 (Blazor Wasm) - ENABLED
With Assets and C# DevKit Installed targetting .NET 9 (Blazor Server Wasm) - ENABLED
Without Assets and C# DevKit Installed targetting .NET 8 (Blazor Wasm) - DISABLED
Without Assets and C# DevKit Installed targetting .NET 8 (Blazor Server Wasm) - DISABLED
Without Assets and C# DevKit Installed targetting .NET 9 (Blazor Wasm) - ENABLED
Without Assets and C# DevKit Installed targetting .NET 9 (Blazor Server Wasm) - ENABLED
Tested scenarios only updating C# DevKit with the monovsdbg support:
Tested scenarios with this PR and C# DevKit without the monovsdbg support:
With Assets and C# DevKit Installed targetting .NET 8 (Blazor Wasm) - DISABLED
With Assets and C# DevKit Installed targetting .NET 8 (Blazor Server Wasm) - DISABLED
With Assets and C# DevKit Installed targetting .NET 9 (Blazor Wasm) - ENABLED
With Assets and C# DevKit Installed targetting .NET 9 (Blazor Server Wasm) - ENABLED
Without Assets and C# DevKit Installed targetting .NET 8 (Blazor Wasm) - DISABLED
Without Assets and C# DevKit Installed targetting .NET 8 (Blazor Server Wasm) - DISABLED
Without Assets and C# DevKit Installed targetting .NET 9 (Blazor Wasm) - DISABLED
Without Assets and C# DevKit Installed targetting .NET 9 (Blazor Server Wasm) - DISABLED