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

[wasm] Fix regression in blazor+aot #31870

Merged
merged 1 commit into from
Apr 19, 2023
Merged

Conversation

radical
Copy link
Member

@radical radical commented Apr 18, 2023

  • For blazor AOT, the blazor targets explicitly skip Microsoft.JSInterop.WebAssembly.dll in _GatherBlazorFilesToPublish target.

    • it is done by marking the assembly item in @(WasmAssembliesToBundle) with %(AOT_InternalForceToInterpret)="true".
  • this target needs to run after _GatherWasmFilesToPublish which builds the list of assemblies to AOT in @(WasmAssembliesToBundle)

  • the order of these two targets was corrected in Fix order of targets in WasmNestedPublishAppDependsOn #31640

  • but the changes in Move asset compression to the StaticWebAssets SDK #31559 added the target twice, resulting in the order:

    _GatherBlazorFilesToPublish;_GatherWasmFilesToPublish;_GatherBlazorFilesToPublish

    1. in which the first _GatherBlazorFilesToPublish tries to remove the assembly from an empty list
    2. then _GatherWasmFilesToPublish populates the list
    3. and the last instance of _GatherBlazorFilesToPublish target doesn't run because it has already run before in (2).
  • thus the assembly never gets removed from the list.

Fixes: dotnet/runtime#85010

- For blazor AOT, the blazor targets explicitly skip
  `Microsoft.JSInterop.WebAssembly.dll` in `_GatherBlazorFilesToPublish` target.
  - it is done by marking the assembly item in
    `@(WasmAssembliesToBundle)` with `%(AOT_InternalForceToInterpret)="true"`.

- this target needs to run after `_GatherWasmFilesToPublish` which builds
  the list of assemblies to AOT in `@(WasmAssembliesToBundle)`

- the order of these two targets was corrected in dotnet#31640
- but the changes in dotnet#31559 added the
  target twice, resulting in the order:

  `_GatherBlazorFilesToPublish;_GatherWasmFilesToPublish;_GatherBlazorFilesToPublish`

  1. in which the first `_GatherBlazorFilesToPublish` tries to remove
     the assembly from an empty list
  2. then `_GatherWasmFilesToPublish` populates the list
  3. and the last instance of `_GatherBlazorFilesToPublish` target
     doesn't run because it has already run before in (2).
- thus the assembly never gets removed from the list.

Fixes: dotnet/runtime#85010
@radical radical requested a review from a team as a code owner April 18, 2023 21:48
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch untriaged Request triage from a team member labels Apr 18, 2023
@ghost
Copy link

ghost commented Apr 18, 2023

Thanks for your PR, @radical.
To learn about the PR process and branching schedule of this repo, please take a look at the SDK PR Guide.

@radical radical requested a review from javiercn April 18, 2023 22:08
@radical radical enabled auto-merge April 18, 2023 23:56
@radical radical merged commit 79b198d into dotnet:main Apr 19, 2023
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 untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[wasm] Blazor/AOT tests failing because Microsoft.JSInterop.WebAssembly is not getting skipped for AOT
2 participants