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] feat: Make boot json file configurable #108281

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jeromelaban
Copy link
Contributor

This change makes the blazor.boot.json file configurable from msbuild, using WasmBuildBootJsonFileName and WasmBuildPublishBootJsonFileName.

@jeromelaban jeromelaban marked this pull request as ready for review September 26, 2024 13:05
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Sep 26, 2024
@jeromelaban jeromelaban changed the title feat: Make boot json file configurable [Wasm] feat: Make boot json file configurable Sep 26, 2024
@lewing lewing requested a review from maraf September 26, 2024 15:09
@@ -19,6 +19,8 @@ Copyright (c) .NET Foundation. All rights reserved.
<RunCommand Condition="'$(RunCommand)' == ''">dotnet</RunCommand>

<WasmAppHostDir>$([MSBuild]::NormalizeDirectory($(MSBuildThisFileDirectory), '..', 'WasmAppHost'))</WasmAppHostDir>
<WasmBuildBootJsonFileName Condition="'$(WasmBuildBootJsonFileName)'==''">blazor.boot.json</WasmBuildBootJsonFileName>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer WasmBootConfigFileName property name

@@ -19,6 +19,8 @@ Copyright (c) .NET Foundation. All rights reserved.
<RunCommand Condition="'$(RunCommand)' == ''">dotnet</RunCommand>

<WasmAppHostDir>$([MSBuild]::NormalizeDirectory($(MSBuildThisFileDirectory), '..', 'WasmAppHost'))</WasmAppHostDir>
<WasmBuildBootJsonFileName Condition="'$(WasmBuildBootJsonFileName)'==''">blazor.boot.json</WasmBuildBootJsonFileName>
<WasmBuildPublishBootJsonFileName Condition="'$(WasmBuildPublishBootJsonFileName)'==''">blazor.publish.boot.json</WasmBuildPublishBootJsonFileName>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this property is used only for intermediate file, we can compute the file name for publish based on WasmBootConfigFileName and skip the need property, or at least for "public property". Something like publish.$(WasmBootConfigFileName) should be enough

@@ -589,8 +591,8 @@ Copyright (c) .NET Foundation. All rights reserved.
<Target Name="_AddPublishWasmBootJsonToStaticWebAssets" DependsOnTargets="GeneratePublishWasmBootJson">
<ItemGroup>
<_PublishWasmBootJson
Include="$(IntermediateOutputPath)blazor.publish.boot.json"
RelativePath="_framework/blazor.boot.json" />
Include="$(IntermediateOutputPath)$(WasmBuildPublishBootJsonFileName)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two other places where blazor.publish.boot.json is used

@maraf maraf added this to the 10.0.0 milestone Sep 27, 2024
@maraf maraf added arch-wasm WebAssembly architecture os-browser Browser variant of arch-wasm labels Sep 27, 2024
Copy link
Contributor

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm WebAssembly architecture area-Build-mono community-contribution Indicates that the PR has been added by a community member os-browser Browser variant of arch-wasm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants