Ship blazor.web.js with the app instead of reading the build machines cache #4
Loading…
Reference in a new issue
No description provided.
Delete branch "b/blazor-web-js-clean-machine"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Fixes the blank white window on machines that never had the .NET SDK. blazor.web.js now reaches wwwroot/_framework three ways: as Content that survives dotnet publish, from the restored Internal.Assets package at startup, and from a copy embedded in the library that is also served directly when the install directory is read-only.
Also adds a release-time version pin check: 3.4.0 published with the templates still pinned to 3.3.1, so pack produced a 3.3.1 template package and --skip-duplicate dropped it silently.
🤖 Claude AI Code Review
Code Review: Ship blazor.web.js with the app
1. Summary
This PR fixes a blank white window on machines without the .NET SDK/NuGet cache by moving
blazor.web.jsfrom a runtime cache scrape to a three-layer delivery strategy:dotnet publish) sourced from the restoredInternal.AssetspackageIt also adds a CI version-pin guard (motivated by a real 3.4.0 partial-release incident) and bumps versions to 3.4.1.
Overall this is a well-engineered, thoughtfully documented fix. The defense-in-depth approach is sound, the comments explain why rather than what, and the tests are meaningful.
2. Code Quality
Strengths:
UsingMicrosoftNETSdkWeb && OutputType==Exe) is exactly the kind of tribal knowledge that should be in code.EnsureBlazorWebJsIsEmbeddedbuild-time<Error>guard is a great touch — it converts a silent empty-embed into a hard failure.3.4.10vs3.4.1shows careful thought.Minor observations:
\ufeff) additions across many files are noise, likely from an editor re-saving. Harmless but pollutes the diff. Worth confirming they're intentional/consistent with.editorconfig.EmbeddedResource Includeuses$(Pkg...)path property; correct givenGeneratePathProperty="true", and the guard handles the empty case.3. Potential Issues
3.1 Content targeting
BeforeTargets="AssignTargetPaths"(verify)Changing from
AfterTargets="Build"toBeforeTargets="AssignTargetPaths"is necessary so theContentitem participates in the standard content pipeline. Confirm$(BlazorFrameworkStaticWebAssetRoot)is already defined at that point in evaluation order — if the Internal.Assets targets set it later,_CheapBlazorWebJsSourcewill be empty and silently fall through to layer 2/3. Not a correctness bug (fallbacks exist), but it would defeat layer 1 for publish. Worth a quick manualdotnet publishverification that the file lands in the publish dir.3.2 Middleware stream length / seekability
Manifest resource streams are seekable, so
ContentLengthwill be set — good. No issue, just noting the guard is effectively always true here.3.3 Middleware missing 304 / conditional request handling
The fallback middleware sets
CacheControl = "no-cache"and always writes the full body. It does not honorIf-None-Match/If-Modified-Since, and doesn't setContent-Lengthwhen non-seekable. Since this is a last-resort path serving a ~single small-ish file, this is acceptable, butno-cachecombined with WebView2's aggressive caching means it re-downloads every navigation. Acceptable given this only runs in the rare read-only-install case.3.4 Middleware vs. antiforgery/short-circuit ordering
The middleware calls
await next()for non-matching requests — correct. But it terminates the pipeline for matching requests without callingnext(), which is intended. Confirm this doesn't sit before any middleware that must run for all requests (it's registered right after static files, so fine).3.5
EmbeddedBlazorHostServicefully-qualified callWorks, but calling an
internal staticextension method via full qualification instead ofapp.UseEmbeddedBlazorFrameworkFallback()is slightly inconsistent with the extension-method style used elsewhere. Cosmetic.3.6 Version skew on layer 3 (documented, acceptable)
The embedded copy is frozen at pack time. The docs correctly note protocol stability within a major version. This is a reasonable tradeoff and clearly disclosed. No action needed, but worth flagging that a future major-version runtime mismatch on a cache-less machine would be a real (if unlikely) failure — the embedded-resource-only path has no version negotiation safety net.
3.7 CI check coverage gap
The version-pin check lists specific files. If a new packable
.csprojis added later, it won't be checked automatically. Consider globbing or a comment reminding maintainers to add new files. Minor.4. Recommendations
dotnet publishactually placeswwwroot/_framework/blazor.web.jsin the publish output. This is the primary fix; the tests assert the targets XML shape but not the end-to-end publish result.app.UseEmbeddedBlazorFrameworkFallback()call style inEmbeddedBlazorHostServicefor consistency.**/*.csprojfiltered to packable projects) to avoid future silent gaps like the one this guard exists to prevent.5. Verdict
APPROVE (with minor suggestions)
This is a high-quality fix addressing a real, hard-to-diagnose production failure. The layered approach is robust, the build-time guard prevents the empty-embed footgun, the CI version check directly addresses the incident that caused the botched 3.4.0 release, and the tests assert the new behavior meaningfully. The recommendations above are non-blocking; the only thing I'd genuinely want before merge is confidence (manual or automated) that layer 1 actually survives
dotnet publish, since that's the mainline path and it's currently only verified at the XML level.📊 Tokens: 16985 input + 2328 output | 💰 Cost: ~$0.1431
Generated by Claude Opus 4.8 via Gitea Actions
forgejo-actions referenced this pull request2026-08-26 14:21:22 +00:00