-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Do not include the JS as Content #32783
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
The TS compiler automatically includes the JS files as content, but we want it as embedded files. Fixes #32683
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 an issue where the TypeScript compiler was automatically including generated JavaScript files as Content items, when they should be embedded as resources in the assembly. The fix ensures that HybridWebView.js (generated from HybridWebView.ts) is properly embedded rather than included as content.
Key Changes
- Moved TypeScript compilation settings to
Directory.Build.propsfor global availability - Added MSBuild target override to prevent TypeScript compiler from including JS files as Content
- Added explicit
<None Remove>directive to exclude the generated JS file from content inclusion
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Directory.Build.props | Moved TypeScript compiler configuration from Core.csproj to make settings globally available |
| Directory.Build.targets | Added GetTypeScriptOutputForPublishing target override to prevent JS files from being included as Content |
| src/Core/src/Core.csproj | Removed TypeScript configuration (moved to Directory.Build.props) and added <None Remove> to exclude HybridWebView.js from content |
| src/TestUtils/src/Microsoft.Maui.IntegrationTests/WindowsTemplateTest.cs | Added integration test to verify the fix works with Microsoft.Identity.Client packages |
| Assert.IsTrue(DotnetInternal.New("maui", projectDir, DotNetCurrent), | ||
| $"Unable to create template maui. Check test output for errors."); | ||
|
|
||
| // .NET 9 and later was Unpackaged, so we need to remove the line |
Copilot
AI
Nov 21, 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 comment is misleading. The test is adding package references to reproduce issue #32683, not removing the WindowsPackageType property. Consider updating it to something like:
// Add Microsoft.Identity.Client packages to test that JS files are properly embedded| // .NET 9 and later was Unpackaged, so we need to remove the line | |
| // Add Microsoft.Identity.Client packages to test that JS files are properly embedded |
src/TestUtils/src/Microsoft.Maui.IntegrationTests/WindowsTemplateTest.cs
Show resolved
Hide resolved
|
@mattleibow this keeps failing the build. The Copilot suggestion looks valid, so applied that... 🤞 |
…ateTest.cs Co-authored-by: Copilot <[email protected]>
|
@jfversluis Thanks! I am now mirroring the packages to hopefully get it all green. |
DevDachi
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.
Thanks for taking care of this!
|
@DevDachi did you actually test the artifacts from this build? |
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description of Change
The TS compiler automatically includes the JS files as content, but we want it as embedded files.
Issues Fixed
Fixes #32683