-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: use asset fingerprinting #144
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
|
Caution Review failedThe pull request is closed. WalkthroughUpdated build and project configs to .NET 10.0, added Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CI as CI / Build Script
participant Installer as dotnet-install.sh
participant Publisher as dotnet publish
participant Host as Hosting/Publish Pipeline
participant Browser as Client Browser
participant Console as DevTools Console
Note over CI,Installer: Build-time changes (SDK upgrade, placeholders)
CI->>Installer: download & run (channel 10.0)
Installer-->>CI: .NET 10 installed
CI->>Publisher: publish Blazor WASM (Release) with asset fingerprint placeholders
Publisher-->>Host: published assets (placeholders present)
Note over Browser: Runtime (placeholder resolution by host)
Browser->>Host: request `index.html` (contains fingerprint placeholders, importmap, preload)
Host-->>Browser: `index.html` (resolved asset URLs)
Browser->>Browser: fetch assets (JS/CSS)
Browser->>Console: execute `wwwroot/js/app.js` → styled console message shown
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cloudflare-build.sh (1)
5-5: Install .NET 10.0 SDK
The installation step has been updated correctly. Consider addingset -euo pipefailat the top of the script to ensure it fails fast on any error.src/Meenzen.SteamTools/wwwroot/index.html (1)
14-14: Empty<script type="importmap">tag
This import map is currently empty. If it’s intended for future dynamic imports, consider adding aTODOcomment or removing it until it's populated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.config/dotnet-tools.json(1 hunks)cloudflare-build.sh(1 hunks)src/Meenzen.SteamTools.ApiProxy/Meenzen.SteamTools.ApiProxy.csproj(1 hunks)src/Meenzen.SteamTools/Meenzen.SteamTools.csproj(1 hunks)src/Meenzen.SteamTools/wwwroot/index.html(2 hunks)
🔇 Additional comments (7)
src/Meenzen.SteamTools.ApiProxy/Meenzen.SteamTools.ApiProxy.csproj (1)
3-3: Upgrade TargetFramework to .NET 10.0
Updating the target framework aligns this project with the rest of the solution.src/Meenzen.SteamTools/Meenzen.SteamTools.csproj (2)
3-3: Upgrade TargetFramework to .NET 10.0
This change ensures the Blazor WebAssembly project builds against the latest runtime.
9-9: Add<OverrideHtmlAssetPlaceholders>property
Ensure theOverrideHtmlAssetPlaceholdersMSBuild property is supported in .NET 10.0 and properly replaces your placeholder tokens during publish..config/dotnet-tools.json (2)
7-10: Enable tool roll-forward forhusky
Defining"rollForward": trueallows patch-level roll-forward while respecting your pinned version. Verify this meets your CI stability requirements.
14-17: Enable tool roll-forward forcsharpier
Similarly, allowing roll-forward forcsharpieris a good practice—confirm that this aligns with your formatting and versioning policies.src/Meenzen.SteamTools/wwwroot/index.html (2)
11-12: Use dynamic fingerprint placeholders for CSS assets
Confirm that the placeholder syntax#[.{fingerprint}]matches the MSBuild asset injection format and thatOverrideHtmlAssetPlaceholderswill replace these correctly at publish time.
34-35: Use dynamic fingerprint placeholders for JS assets
Ensure that the fingerprint placeholders in the<script>srcattributes are correctly replaced by the build and that the load order (framework first, then MudBlazor) remains intact.
Deploying steam-tools with
|
| Latest commit: |
5cab908
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://791d72ce.steam-tools-8yd.pages.dev |
| Branch Preview URL: | https://feat-asset-fingerprinting.steam-tools-8yd.pages.dev |
071e55f to
b5bac44
Compare
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Meenzen.SteamTools/Meenzen.SteamTools.csproj (1)
9-11: Consider adding patterns for fonts/images if referenced directly in HTML.If you serve favicon, SVG, or font files via HTML tags, add patterns so they cache-bust too. Safe to skip if they’re only referenced from CSS (those URLs typically already include hashes).
Example:
<ItemGroup> <StaticWebAssetFingerprintPattern Include="JS" Pattern="*.js" Expression="#[.{fingerprint}]!" /> <StaticWebAssetFingerprintPattern Include="CSS" Pattern="*.css" Expression="#[.{fingerprint}]!" /> + <StaticWebAssetFingerprintPattern Include="FONTS" Pattern="*.{woff,woff2,ttf,otf}" Expression="#[.{fingerprint}]!" /> + <StaticWebAssetFingerprintPattern Include="IMAGES" Pattern="*.{png,svg,ico,webp,avif}" Expression="#[.{fingerprint}]!" /> </ItemGroup>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.config/dotnet-tools.json(1 hunks)cloudflare-build.sh(1 hunks)src/Meenzen.SteamTools.ApiProxy/Meenzen.SteamTools.ApiProxy.csproj(1 hunks)src/Meenzen.SteamTools/Meenzen.SteamTools.csproj(1 hunks)src/Meenzen.SteamTools/wwwroot/index.html(2 hunks)src/Meenzen.SteamTools/wwwroot/js/app.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/Meenzen.SteamTools.ApiProxy/Meenzen.SteamTools.ApiProxy.csproj
- cloudflare-build.sh
- src/Meenzen.SteamTools/wwwroot/js/app.js
- src/Meenzen.SteamTools/wwwroot/index.html
- .config/dotnet-tools.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
src/Meenzen.SteamTools/Meenzen.SteamTools.csproj (2)
15-20: Align pre-release Blazor packages with your SDK and plan RTM bump
You’re on 10.0.0-rc.1 for both Components.WebAssembly and DevServer—manually verify that your local and CI .NET SDK band matches rc.1 (and that your Cloudflare build uses the same channel), then create a follow-up to upgrade to 10.0.0 RTM once it’s released.
3-3: Verify .NET 10 RC upgrade necessity and CI/hosting support
Asset placeholders and fingerprinting have been available since .NET 9; if you don’t strictly need net10.0-rc, stay on a stable TFM until 10.0 GA. Confirm your CI/build image and hosting environment support .NET 10 by runningdotnet --infoin your pipeline and inspecting any global.json or SDK version pins.
Summary by CodeRabbit
New Features
Chores