feat: Add indexAssets() and tryAssetPathSync() to CacheBustingConfig#332
feat: Add indexAssets() and tryAssetPathSync() to CacheBustingConfig#332nielsenko merged 4 commits intoserverpod:mainfrom
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR extends Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
|
@CodeRabbit review |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #332 +/- ##
==========================================
+ Coverage 91.44% 91.45% +0.01%
==========================================
Files 97 97
Lines 3797 3816 +19
Branches 1935 1943 +8
==========================================
+ Hits 3472 3490 +18
- Misses 325 326 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/relic_io/lib/src/io/static/cache_busting_config.dart (1)
121-138: Consider clearing_cacheat the start ofindexAssets.If
indexAssets()is called more than once (e.g., after a file is deleted or renamed), stale entries from the previous run will persist because the cache is never cleared. If this is intended to be called only once at startup, a doc comment noting that would be helpful; otherwise, adding_cache.clear()at the top would make repeated calls idempotent.Suggested fix
Future<void> indexAssets() async { + _cache.clear(); final resolvedRootPath = fileSystemRoot.resolveSymbolicLinksSync();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/relic_io/lib/src/io/static/cache_busting_config.dart` around lines 121 - 138, The indexAssets method can leave stale entries when run multiple times; make it idempotent by clearing the in-memory cache at the start of indexAssets (call _cache.clear()) so previous runs' entries are removed before re-indexing, or if intended to be one-time only add a doc comment to indexAssets explaining it must only be called once; locate the indexAssets function and add the _cache.clear() call as the first action (or add the explanatory doc comment above the indexAssets declaration).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/relic_io/lib/src/io/static/cache_busting_config.dart`:
- Around line 121-138: The indexAssets method can leave stale entries when run
multiple times; make it idempotent by clearing the in-memory cache at the start
of indexAssets (call _cache.clear()) so previous runs' entries are removed
before re-indexing, or if intended to be one-time only add a doc comment to
indexAssets explaining it must only be called once; locate the indexAssets
function and add the _cache.clear() call as the first action (or add the
explanatory doc comment above the indexAssets declaration).
d65b0c0 to
e8a2ea0
Compare
e8a2ea0 to
1ae1300
Compare
marcelomendoncasoares
left a comment
There was a problem hiding this comment.
Just one minor question and suggestion on the tests, but LGTM!
packages/relic_io/test/static/cache_busting_static_handler_test.dart
Outdated
Show resolved
Hide resolved
9baef59 to
8f4409f
Compare
Description
Adds synchronous path lookups to
CacheBustingConfig, enabling integration with synchronous template engines (Mustache lambdas, Ninja filters, etc.).indexAssets()toCacheBustingConfig- walksfileSystemRoot, computes hashes, populates an internal cachetryAssetPathSync()- sync cache lookup, returns original path if not cachedassetPath()andtryAssetPath()now populate the cache as a side-effectStaticHandler.directory()automatically callsindexAssets()when acacheBustingConfigis provided; the future is awaited on first requestRelated Issues
Pre-Launch Checklist
///).Breaking Changes
Additional Notes
None.
Summary by CodeRabbit