-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix compression input file resolution for esproj assets #52283
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
Conversation
|
Thanks for your PR, @@javiercn. |
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
This PR fixes a critical bug in the compression of static web assets from esproj projects. Previously, compressed files contained XML content from the .esproj project file instead of the actual JavaScript content because the compression task used the wrong file path.
Key Changes:
- Reordered file path resolution to prefer
RelatedAsset(the asset's Identity) overRelatedAssetOriginalItemSpec(which may incorrectly reference the project file) - Added comprehensive unit tests covering the new resolution order and edge cases
- Updated error message parameter order to match the new check sequence
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/StaticWebAssetsSdk/Tasks/Utils/AssetToCompress.cs |
Changed TryFindInputFilePath to check RelatedAsset first before falling back to RelatedAssetOriginalItemSpec, and updated error message parameter order to reflect the new sequence |
test/Microsoft.NET.Sdk.StaticWebAssets.Tests/StaticWebAssets/AssetToCompressTest.cs |
Added new test class with 6 comprehensive test cases verifying the corrected file resolution order, fallback behavior, error handling, and the specific esproj scenario |
When compressing static web assets from esproj projects, the compression task incorrectly used the .esproj project file as the input instead of the actual JavaScript asset file. This happened because the code checked RelatedAssetOriginalItemSpec before RelatedAsset. The esproj SDK has a bug where OriginalItemSpec gets set to the project file path instead of the asset file path (due to referencing %(CandidatePublishAssets.Identity) instead of %(CandidateBuildAssets.Identity) in the GetCurrentProjectBuildStaticWebAssetItems target). This fix changes the order of checks in TryFindInputFilePath to prefer RelatedAsset (the asset's Identity path) over RelatedAssetOriginalItemSpec. RelatedAsset contains the correct file path even when OriginalItemSpec is incorrect. Symptoms this fixes: - Compressed files (.gz/.br) containing XML content of .esproj file - JavaScript interop failures in Blazor apps using esproj-based libraries Added unit tests to verify: - RelatedAsset is preferred when both paths exist - Fallback to RelatedAssetOriginalItemSpec when RelatedAsset doesn't exist - Error handling when neither path exists - Specific esproj scenario with project file vs JS file
702af2d to
bf590b9
Compare
MackinnonBuck
left a comment
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.
Looks good!
| // Check RelatedAsset first (the asset's Identity path) as it's more reliable. | ||
| // RelatedAssetOriginalItemSpec may point to a project file (e.g., .esproj) rather than the actual asset. | ||
| var relatedAsset = assetToCompress.GetMetadata("RelatedAsset"); |
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.
Is this more reliable in the general case or only as it relates to the esproj SDK?
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.
Yeah, it is in general more reliable to first look if the file exists first for the related asset and the only fallback to the original spec if it doesn't. This logic is there because of the trick we have to do for webassembly apps.
Summary
When compressing static web assets from esproj projects, the compression task incorrectly used the
.esprojproject file as the input instead of the actual JavaScript asset file. This resulted in compressed files (.gz/.br) containing XML content instead of compressed JavaScript.Root Cause
The
TryFindInputFilePathmethod inAssetToCompress.cscheckedRelatedAssetOriginalItemSpecbeforeRelatedAsset. The esproj SDK has a bug whereOriginalItemSpecgets set to the project file path instead of the asset file path (due to referencing%(CandidatePublishAssets.Identity)instead of%(CandidateBuildAssets.Identity)in theGetCurrentProjectBuildStaticWebAssetItemstarget).Fix
This PR changes the order of checks in
TryFindInputFilePathto preferRelatedAsset(the asset's Identity path) overRelatedAssetOriginalItemSpec.RelatedAssetcontains the correct file path even whenOriginalItemSpecis incorrect.Symptoms this fixes
.gz/.br) containing XML content of.esprojfile instead of compressed JavaScriptTesting
Added unit tests to verify:
RelatedAssetis preferred when both paths existRelatedAssetOriginalItemSpecwhenRelatedAssetdoesn't existAll existing tests continue to pass.
#Fixes #50987