Skip to content
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

[Static web assets] Fixes issues we discovered while integrating the static web assets improvements on to the ASP.NET Core repo #19080

Merged
merged 4 commits into from
Jul 22, 2021

Conversation

javiercn
Copy link
Member

@javiercn javiercn commented Jul 20, 2021

Description

As part of integrating the changes we did to static web assets into the ASP.NET Core repo, we ran into some issues and this PR Contains the fixes to them.

  • Filter doc XML files from blazor static web assets
  • Fallback to Identity metadata when we try to match project references
    • This is not technically necessary because regular projects have the right metadata, however, ASP.NET Core uses some
      special project types and its easy/cheap for us to be more lenient here and fallback.
  • Correct empty entry for manifest pattern on generation
    • The dotnet.js manifest file was not being passed on to GenerateBlazorBootJson manifest.
  • When generating the manifest we use for development we were including empty entries "" as well as the base path for assets
    in the current project, which is not necessary.
  • Only include discovery patterns when the folder exists on disk
    • For example, avoid creating a pattern when the wwwroot folder doesn't exist, since we can't map a non existing folder at runtime.
  • Correctly compute the asset path for assets that are copied to the output folder.

The PR is divided in three commits:

  • The first commit is the important one and contains the fixes mentioned above.
  • The second commit contains test improvements we've done to ensure we are solid.
  • The third commit updates the test baselines for the different builds we perform.

Customer Impact

Customers won't be able to load Blazor apps unless they switch to the previous Static Web Assets implementation (we included a flag for this) however, we want to make sure this is the default experience in Preview7 to ensure that we uncover as many issues as we can, since these improvements are the foundation for many of the new features we are building

Regression?

  • Yes
  • No

6.0 Preview6

Risk

  • High
  • Medium
  • Low

The changes are complex, however we put a fallback mechanism in place to force the older implementation to be used. In case a scenario doesn't work for a user, they can resort to the preview6 implementation setting a value on their csproj file.

Verification

  • Manual (required)
  • Automated

Manual verification is problematic and limited due to the fact that there is a build time component and a runtime component and that they live on different repositories.

I've been able to use a freshly built "dev sdk" to build one of the samples and i've used the runtime changes (paired with this ones) to run the project and run the published project.

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@javiercn javiercn added the Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch label Jul 20, 2021
@javiercn javiercn force-pushed the javiercn/static-web-assets-fixes branch 2 times, most recently from 3f42b37 to 388a0a8 Compare July 20, 2021 20:42
@javiercn javiercn marked this pull request as ready for review July 20, 2021 20:49
@javiercn javiercn requested a review from pranavkm July 20, 2021 20:49
@mkArtakMSFT
Copy link
Member

Approved by tactics.

@javiercn javiercn merged commit 3ddd061 into release/6.0.1xx-preview7 Jul 22, 2021
javiercn added a commit that referenced this pull request Jul 24, 2021
…static web assets improvements on to the ASP.NET Core repo (#19080)

* Filter doc XML files from blazor static web assets
* Fallback to Identity metadata when we try to match project references
  * This is not technically necessary because regular projects have the right metadata, however, ASP.NET Core uses some
special project types and its easy/cheap for us to be more lenient here and fallback.
* Correct empty entry for manifest pattern on generation
* The dotnet.js manifest file was not being passed on to GenerateBlazorBootJson manifest.
* When generating the manifest we use for development we were including empty entries "" as well as the base path for assets in the current project, which is not necessary.
* Only include discovery patterns when the folder exists on disk
  * For example, avoid creating a pattern when the wwwroot folder doesn't exist, since we can't map a non existing folder at runtime.
* Correctly compute the asset path for assets that are copied to the output folder.
* Fix how we deal with transitive assets
javiercn added a commit that referenced this pull request Jul 24, 2021
* [Static web assets] Fixes issues we discovered while integrating the static web assets improvements on to the ASP.NET Core repo (#19080)
* [Static web assets] Fixes issue with 3.1 and 5.0 wasm apps (#19172)
@akoeplinger akoeplinger deleted the javiercn/static-web-assets-fixes branch February 14, 2024 14:19
v-wuzhai pushed a commit that referenced this pull request Apr 22, 2024
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: JL03-Yue <59816815+JL03-Yue@users.noreply.github.com>
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch Servicing-approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants