Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
📝 WalkthroughWalkthroughModified static file serving configuration in Startup.cs to transition from legacy WebOptimizer bundling to Vite ESM asset serving under Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Web/Resgrid.Web/Startup.cs`:
- Around line 596-603: The cache policy currently marks only "/js/ng/chunks" as
immutable; update the OnPrepareResponse logic to also treat "/js/ng/assets" as
immutable by checking both path segments (e.g., include "/js/ng/assets"
alongside "/js/ng/chunks" when computing cacheControl) so fingerprinted assets
under assets/[name]-[hash][extname] get
"public,max-age=31536000,immutable,no-transform" while non-hashed entries like
react-elements.js remain "no-cache,no-transform"; modify the conditional in the
OnPrepareResponse delegate accordingly.
- Around line 592-605: The Cache-Control decision in the OnPrepareResponse
lambda for clientApp.UseStaticFiles only marks "/js/ng/chunks" as immutable;
update the conditional in that lambda (the cacheControl assignment using
context.Context.Request.Path.StartsWithSegments) to also treat "/js/ng/assets"
as long-lived (e.g., use StartsWithSegments("/js/ng/chunks") ||
StartsWithSegments("/js/ng/assets")) so both content-hashed chunks and assets
get "public,max-age=31536000,immutable,no-transform" while leaving unhashed
entries (like react-elements.js) using "no-cache,no-transform".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 85f11027-d174-4b20-abca-a3289e60ecd9
📒 Files selected for processing (1)
Web/Resgrid.Web/Startup.cs
| clientApp.UseStaticFiles(new StaticFileOptions | ||
| { | ||
| FileProvider = new PhysicalFileProvider(Path.Combine(env.WebRootPath, "js", "ng")), | ||
| RequestPath = "/js/ng", | ||
| OnPrepareResponse = context => | ||
| { | ||
| var cacheControl = context.Context.Request.Path.StartsWithSegments("/js/ng/chunks") | ||
| ? "public,max-age=31536000,immutable,no-transform" | ||
| : "no-cache,no-transform"; | ||
|
|
||
| context.Context.Response.Headers["Cache-Control"] = cacheControl; | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether the Vite output directory is expected to always exist at Configure-time
# (e.g. checked in, produced by an msbuild target, or a pre-run step).
fd -t d -u '^ng$' | rg -n 'wwwroot/js/ng'
rg -nP --type=xml -C2 '(js[/\\\\]ng|vite|npm (run )?build)' -g '*.csproj'
rg -nP -C2 '(js[/\\\\]ng|vite\s+build|npm\s+run\s+build)' -g '!node_modules' -g '!*.min.*' -g '!*.map'Repository: Resgrid/Core
Length of output: 6521
🏁 Script executed:
#!/bin/bash
# Check if wwwroot/js/ng exists in git (not gitignored)
git ls-files | grep -E 'wwwroot/js/ng'
# Check .gitignore patterns that might exclude this directory
cat .gitignore 2>/dev/null | grep -E '(wwwroot|js|ng|/dist)' || echo "No gitignore patterns found matching"
# Check if wwwroot itself is gitignored
git check-ignore -v 'Web/Resgrid.Web/wwwroot' 2>/dev/null || echo "wwwroot is not gitignored"
git check-ignore -v 'Web/Resgrid.Web/wwwroot/js/ng' 2>/dev/null || echo "wwwroot/js/ng is not gitignored"
# List what's actually in the directory in git
echo "=== Files in wwwroot/js/ng in git ==="
git ls-files | grep 'wwwroot/js/ng' | head -20 || echo "No files found in git"Repository: Resgrid/Core
Length of output: 2726
Cache-Control headers should apply immutable to all content-hashed assets, not just chunks.
Per vite.config.ts, both /js/ng/chunks/ and /js/ng/assets/ output are content-hashed ([name]-[hash] pattern), making them safe for long-lived caching. Currently only chunks receive immutable, max-age=31536000; assets still get no-cache, which undermines the caching benefit of the hash. The unhashed entry react-elements.js correctly stays at no-cache.
Update the cache-control logic to treat all hashed assets uniformly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web/Startup.cs` around lines 592 - 605, The Cache-Control
decision in the OnPrepareResponse lambda for clientApp.UseStaticFiles only marks
"/js/ng/chunks" as immutable; update the conditional in that lambda (the
cacheControl assignment using context.Context.Request.Path.StartsWithSegments)
to also treat "/js/ng/assets" as long-lived (e.g., use
StartsWithSegments("/js/ng/chunks") || StartsWithSegments("/js/ng/assets")) so
both content-hashed chunks and assets get
"public,max-age=31536000,immutable,no-transform" while leaving unhashed entries
(like react-elements.js) using "no-cache,no-transform".
| OnPrepareResponse = context => | ||
| { | ||
| var cacheControl = context.Context.Request.Path.StartsWithSegments("/js/ng/chunks") | ||
| ? "public,max-age=31536000,immutable,no-transform" | ||
| : "no-cache,no-transform"; | ||
|
|
||
| context.Context.Response.Headers["Cache-Control"] = cacheControl; | ||
| } |
There was a problem hiding this comment.
Hashed /js/ng/assets/* files are missing the immutable cache policy.
The vite.config.ts referenced in context emits three output groups, but only one of them is being treated as immutable here:
react-elements.js— entry, not hashed →no-cache✅ correctchunks/[name]-[hash].js— hashed →immutable✅ correctassets/[name]-[hash][extname]— hashed → currentlyno-cache❌
/js/ng/assets/* is content-hashed just like chunks/*, so it's equally safe for long-term immutable caching. As written, browsers will revalidate those fingerprinted assets on every navigation, which negates the hashing strategy.
♻️ Suggested tweak
OnPrepareResponse = context =>
{
- var cacheControl = context.Context.Request.Path.StartsWithSegments("/js/ng/chunks")
+ var path = context.Context.Request.Path;
+ var isHashed = path.StartsWithSegments("/js/ng/chunks")
+ || path.StartsWithSegments("/js/ng/assets");
+ var cacheControl = isHashed
? "public,max-age=31536000,immutable,no-transform"
: "no-cache,no-transform";
context.Context.Response.Headers["Cache-Control"] = cacheControl;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| OnPrepareResponse = context => | |
| { | |
| var cacheControl = context.Context.Request.Path.StartsWithSegments("/js/ng/chunks") | |
| ? "public,max-age=31536000,immutable,no-transform" | |
| : "no-cache,no-transform"; | |
| context.Context.Response.Headers["Cache-Control"] = cacheControl; | |
| } | |
| OnPrepareResponse = context => | |
| { | |
| var path = context.Context.Request.Path; | |
| var isHashed = path.StartsWithSegments("/js/ng/chunks") | |
| || path.StartsWithSegments("/js/ng/assets"); | |
| var cacheControl = isHashed | |
| ? "public,max-age=31536000,immutable,no-transform" | |
| : "no-cache,no-transform"; | |
| context.Context.Response.Headers["Cache-Control"] = cacheControl; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web/Startup.cs` around lines 596 - 603, The cache policy
currently marks only "/js/ng/chunks" as immutable; update the OnPrepareResponse
logic to also treat "/js/ng/assets" as immutable by checking both path segments
(e.g., include "/js/ng/assets" alongside "/js/ng/chunks" when computing
cacheControl) so fingerprinted assets under assets/[name]-[hash][extname] get
"public,max-age=31536000,immutable,no-transform" while non-hashed entries like
react-elements.js remain "no-cache,no-transform"; modify the conditional in the
OnPrepareResponse delegate accordingly.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
1 participant
Summary by CodeRabbit