-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Copydotnet.d.ts
to wwwroot
on build/publish.
#120097
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
Its also worth pointing that
Line 279 in 7ab1ed3
Which is an output from the runtime/src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/ComputeWasmBuildAssets.cs Line 93 in 7ab1ed3
runtime/src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/AssetsComputingHelper.cs Line 74 in 7ab1ed3
|
src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/AssetsComputingHelper.cs
Outdated
Show resolved
Hide resolved
...nuget/Microsoft.NET.Sdk.WebAssembly.Pack/build/Microsoft.NET.Sdk.WebAssembly.Browser.targets
Outdated
Show resolved
Hide resolved
...nuget/Microsoft.NET.Sdk.WebAssembly.Pack/build/Microsoft.NET.Sdk.WebAssembly.Browser.targets
Outdated
Show resolved
Hide resolved
src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/AssetsComputingHelper.cs
Outdated
Show resolved
Hide resolved
src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/ComputeWasmBuildAssets.cs
Outdated
Show resolved
Hide resolved
src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/ComputeWasmBuildAssets.cs
Outdated
Show resolved
Hide resolved
src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/ComputeWasmPublishAssets.cs
Outdated
Show resolved
Hide resolved
src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/ComputeWasmPublishAssets.cs
Outdated
Show resolved
Hide resolved
src/mono/wasm/Wasm.Build.Tests/BrowserStructures/MSBuildOptions.cs
Outdated
Show resolved
Hide resolved
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.
Pull Request Overview
Adds an opt-in MSBuild property to copy the generated dotnet.d.ts TypeScript definition file into a WebAssembly project's wwwroot during build, addressing issue #77174.
- Introduces WasmEmitTypeScriptDefinitions property (default false) in relevant .targets.
- Adds MSBuild target _EnsureDotnetTypeScriptDefinitions to perform the conditional copy.
- Adds a test validating the file copy on build (Debug/Release).
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
src/mono/wasm/build/WasmApp.Common.targets | Documents and defines the new WasmEmitTypeScriptDefinitions property. |
src/mono/nuget/Microsoft.NET.Sdk.WebAssembly.Pack/build/Microsoft.NET.Sdk.WebAssembly.Browser.targets | Adds property default again and new target to copy dotnet.d.ts into wwwroot. |
src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTests.cs | Adds test ensuring dotnet.d.ts is copied on build when the property is enabled. |
<WasmStripILAfterAOT Condition="'$(WasmStripILAfterAOT)' == ''">true</WasmStripILAfterAOT> | ||
<WasmRuntimeAssetsLocation Condition="'$(WasmRuntimeAssetsLocation)' == ''">_framework</WasmRuntimeAssetsLocation> | ||
<MetricsSupport Condition="'$(MetricsSupport)' == ''">false</MetricsSupport> | ||
<WasmEmitTypeScriptDefinitions Condition="'$(WasmEmitTypeScriptDefinitions)' == ''">false</WasmEmitTypeScriptDefinitions> |
Copilot
AI
Oct 3, 2025
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.
The default for WasmEmitTypeScriptDefinitions is also being set in Microsoft.NET.Sdk.WebAssembly.Browser.targets, creating duplicate sources of truth that can drift; consider centralizing the default in a single imported location (or guard one with Condition="'$(WasmEmitTypeScriptDefinitions)'=='' and '$(DefiningFileFlag)'!='true'") to avoid divergence.
<WasmEmitTypeScriptDefinitions Condition="'$(WasmEmitTypeScriptDefinitions)' == ''">false</WasmEmitTypeScriptDefinitions> | |
<WasmEmitTypeScriptDefinitions Condition="'$(WasmEmitTypeScriptDefinitions)' == '' and '$(DefiningFileFlag)' != 'true'">false</WasmEmitTypeScriptDefinitions> |
Copilot uses AI. Check for mistakes.
<_WasmCopyOutputSymbolsToOutputDirectory Condition="'$(_WasmCopyOutputSymbolsToOutputDirectory)'==''">true</_WasmCopyOutputSymbolsToOutputDirectory> | ||
<_WasmEnableThreads>$(WasmEnableThreads)</_WasmEnableThreads> | ||
<_WasmEnableThreads Condition="'$(_WasmEnableThreads)' == ''">false</_WasmEnableThreads> | ||
<WasmEmitTypeScriptDefinitions Condition="'$(WasmEmitTypeScriptDefinitions)' == ''">false</WasmEmitTypeScriptDefinitions> |
Copilot
AI
Oct 3, 2025
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 repeats the default assignment also added in WasmApp.Common.targets; please consolidate the default definition to one place to prevent future inconsistencies.
<WasmEmitTypeScriptDefinitions Condition="'$(WasmEmitTypeScriptDefinitions)' == ''">false</WasmEmitTypeScriptDefinitions> |
Copilot uses AI. Check for mistakes.
<_RuntimePackDir Condition="'$(_RuntimePackDir)' == ''">%(ResolvedRuntimePack.PackageDirectory)</_RuntimePackDir> | ||
<_RuntimePackNativeDir>$([MSBuild]::NormalizeDirectory($(_RuntimePackDir), 'runtimes', 'browser-wasm', 'native'))</_RuntimePackNativeDir> | ||
<_DotnetTypesSourcePath>$(_RuntimePackNativeDir)dotnet.d.ts</_DotnetTypesSourcePath> | ||
<_DotnetTypesDestPath>$(MSBuildProjectDirectory)\wwwroot\dotnet.d.ts</_DotnetTypesDestPath> |
Copilot
AI
Oct 3, 2025
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.
[nitpick] Hardcoded backslashes reduce clarity on non-Windows; prefer a normalized construction such as
<_DotnetTypesDestPath>$(MSBuildProjectDirectory)\wwwroot\dotnet.d.ts</_DotnetTypesDestPath> | |
<_DotnetTypesDestPath>$(MSBuildProjectDirectory)$(DirectorySeparatorChar)wwwroot$(DirectorySeparatorChar)dotnet.d.ts</_DotnetTypesDestPath> |
Copilot uses AI. Check for mistakes.
// Verify dotnet.d.ts is not in wwwroot after creation | ||
Assert.False(File.Exists(dotnetDtsWwwrootPath), $"dotnet.d.ts should not exist at {dotnetDtsWwwrootPath} after creation when WasmEmitTypeScriptDefinitions is used"); | ||
|
||
// Build to trigger the _EnsureDotnetTypeScriptDefinitions target on restore |
Copilot
AI
Oct 3, 2025
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.
The comment is misleading—_EnsureDotnetTypeScriptDefinitions runs AfterTargets _ResolveWasmConfiguration during build, not restore; update the comment to reflect it triggers during the build phase.
// Build to trigger the _EnsureDotnetTypeScriptDefinitions target on restore | |
// Build to trigger the _EnsureDotnetTypeScriptDefinitions target during the build phase |
Copilot uses AI. Check for mistakes.
<WasmStripILAfterAOT Condition="'$(WasmStripILAfterAOT)' == ''">true</WasmStripILAfterAOT> | ||
<WasmRuntimeAssetsLocation Condition="'$(WasmRuntimeAssetsLocation)' == ''">_framework</WasmRuntimeAssetsLocation> | ||
<MetricsSupport Condition="'$(MetricsSupport)' == ''">false</MetricsSupport> | ||
<WasmEmitTypeScriptDefinitions Condition="'$(WasmEmitTypeScriptDefinitions)' == ''">false</WasmEmitTypeScriptDefinitions> |
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.
<WasmEmitTypeScriptDefinitions Condition="'$(WasmEmitTypeScriptDefinitions)' == ''">false</WasmEmitTypeScriptDefinitions> |
There is no need to set the property here as well
<_RuntimePackNativeDir>$([MSBuild]::NormalizeDirectory($(_RuntimePackDir), 'runtimes', 'browser-wasm', 'native'))</_RuntimePackNativeDir> | ||
<_DotnetTypesSourcePath>$(_RuntimePackNativeDir)dotnet.d.ts</_DotnetTypesSourcePath> |
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.
<_RuntimePackNativeDir>$([MSBuild]::NormalizeDirectory($(_RuntimePackDir), 'runtimes', 'browser-wasm', 'native'))</_RuntimePackNativeDir> | |
<_DotnetTypesSourcePath>$(_RuntimePackNativeDir)dotnet.d.ts</_DotnetTypesSourcePath> | |
<_DotnetTypesSourcePath>$([MSBuild]::NormalizeDirectory($(_RuntimePackDir), 'runtimes', 'browser-wasm', 'native', 'dotnet.d.ts'))</_DotnetTypesSourcePath> |
<_WasmCopyOutputSymbolsToOutputDirectory Condition="'$(_WasmCopyOutputSymbolsToOutputDirectory)'==''">true</_WasmCopyOutputSymbolsToOutputDirectory> | ||
<_WasmEnableThreads>$(WasmEnableThreads)</_WasmEnableThreads> | ||
<_WasmEnableThreads Condition="'$(_WasmEnableThreads)' == ''">false</_WasmEnableThreads> | ||
<WasmEmitTypeScriptDefinitions Condition="'$(WasmEmitTypeScriptDefinitions)' == ''">false</WasmEmitTypeScriptDefinitions> |
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.
Please, follow the pattern with assign to private property first, and then set the default value to the private property
[Theory] | ||
[InlineData(Configuration.Debug)] | ||
[InlineData(Configuration.Release)] | ||
public void TypeScriptDefinitionsCopiedToWwwrootOnBuild(Configuration config) |
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.
Please add a test case for false
resulting in file not exist
Fixes #77174.