-
-
Notifications
You must be signed in to change notification settings - Fork 93
refactor: Centralize cache configuration to Stac.initialize and set networkFirst` as default cache strategy.
#435
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
… `networkFirst` as default cache strategy.
…n for version 1.3.0
📝 WalkthroughWalkthroughThe PR centralizes caching into a global configuration via Changes
Sequence Diagram(s)sequenceDiagram
actor App
participant StacInit as "Stac.initialize"
participant StacService as "StacService (global config)"
participant StacCloud as "StacCloud (fetch)"
participant Cache as "Cache Layer"
App->>StacInit: initialize(cacheConfig)
StacInit->>StacService: initialize(cacheConfig)
StacService->>StacService: set defaultCacheConfig
App->>StacCloud: fetchScreen(routeName)
StacCloud->>StacService: get defaultCacheConfig
StacService-->>StacCloud: return config
StacCloud->>Cache: check artifact (uses config)
Cache-->>StacCloud: hit / miss
StacCloud->>StacCloud: network fetch if needed
StacCloud-->>App: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 4
🤖 Fix all issues with AI agents
In `@packages/stac/lib/src/framework/stac.dart`:
- Around line 165-167: The docstring for the cacheConfig parameter is
inconsistent with the actual default; update the parameter documentation to
state the real default used by StacService (networkFirst) so the comment matches
the implementation—locate the cacheConfig parameter's doc in stac.dart and
change "Defaults to optimistic caching if not provided" to "Defaults to
networkFirst if not provided" (referencing StacService and its networkFirst
default).
- Around line 66-67: The doc comment incorrectly states the default caching
strategy is "optimistic" while the implementation/default constant uses
networkFirst; update the comment in stac.dart to reflect the actual default
(networkFirst) so it matches the code—look for symbols like Stac,
defaultCacheStrategy (or any constructor param that sets the default) and the
networkFirst identifier and replace "optimistic" with "networkFirst" (or
rephrase to state that the default is networkFirst) to keep documentation
consistent.
In `@packages/stac/lib/src/models/stac_cache_config.dart`:
- Around line 85-92: The doc comment for the preset fastWithUpdates incorrectly
states "This is the default strategy" while the StacCacheConfig constructor
default is StacCacheStrategy.networkFirst; update the comment on the
fastWithUpdates constant to remove or correct the "default" claim so it
accurately reflects that fastWithUpdates uses StacCacheStrategy.optimistic and
is not the constructor default (alternatively, if intended to be the default,
change the constructor's default strategy to StacCacheStrategy.optimistic in
StacCacheConfig).
In `@packages/stac/README.md`:
- Around line 43-44: The README feature list contains malformed replacement
characters (�) on the two bullets; open the feature list in
packages/stac/README.md and replace the replacement characters with the intended
emojis (e.g., "⚡ Caching: Intelligent screen caching..." and "🔌 Extensible: Add
custom widgets...") and save the file with UTF-8 encoding to prevent recurrence;
confirm the bullets render correctly in the repo viewer.
🧹 Nitpick comments (1)
packages/stac/lib/src/models/stac_cache_config.dart (1)
136-147: Consider:copyWithcannot resetmaxAgetonull.The current implementation uses the
??operator, which means oncemaxAgeis set, it cannot be reset tonullviacopyWith. This is a common Dart pattern and may be acceptable, but worth noting if null-resettability is needed.♻️ Optional fix using sentinel pattern
+ // Sentinel for explicitly setting null + static const _undefined = Object(); + /// Creates a copy of this config with the given fields replaced. StacCacheConfig copyWith({ - Duration? maxAge, + Object? maxAge = _undefined, StacCacheStrategy? strategy, bool? refreshInBackground, }) { return StacCacheConfig( - maxAge: maxAge ?? this.maxAge, + maxAge: maxAge == _undefined ? this.maxAge : maxAge as Duration?, strategy: strategy ?? this.strategy, refreshInBackground: refreshInBackground ?? this.refreshInBackground, ); }
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
🤖 Fix all issues with AI agents
In `@packages/stac/lib/src/models/stac_cache_config.dart`:
- Around line 44-55: The doc examples reference missing static presets
(StacCacheConfig.offlineFirst, .realTime, .fastWithUpdates); either remove those
three example lines from the class-level doc comment, or add the three static
const presets after the refreshInBackground field by defining offlineFirst,
realTime, and fastWithUpdates as StacCacheConfig instances (use
StacCacheStrategy values and sensible Duration / refreshInBackground settings)
so the symbols referenced in the docs actually exist; update the doc comment
accordingly to match the chosen approach.
…, and add an `optimistic` cache strategy to the movie app example.
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
🤖 Fix all issues with AI agents
In `@docs/concepts/caching.mdx`:
- Around line 35-73: Update the Optimistic caching warning copy to avoid
implying updates only appear after an app relaunch; instead state that cached
data is returned immediately and the background refresh will update the cache so
fresh content will be visible on the next load once the refresh completes. Edit
the Warning block under the StacCacheConfig / StacCacheStrategy.optimistic
example to replace the current “until the next app launch” phrasing with
language indicating the cache is refreshed in the background and the updated
content will be shown on the next view/load after the refresh finishes.
| ### 1. Network First (Default) | ||
|
|
||
| Returns cached data immediately while fetching updates in the background. Best for fast perceived performance. | ||
| Always tries the network first, falls back to cache if network fails. **This is the default strategy.** | ||
|
|
||
| ```dart | ||
| Stac( | ||
| routeName: '/home', | ||
| cacheConfig: StacCacheConfig( | ||
| strategy: StacCacheStrategy.optimistic, | ||
| ), | ||
| cacheConfig: StacCacheConfig( | ||
| strategy: StacCacheStrategy.networkFirst, | ||
| ) | ||
| ``` | ||
|
|
||
| **Behavior:** | ||
| - ✅ Always fetches fresh data first | ||
| - ✅ Falls back to cache on network error | ||
| - ✅ Ensures latest content when online | ||
| - 🌐 Requires network for best experience | ||
|
|
||
| **Best for:** Server-driven UI where users should see the latest content, real-time data, frequently changing screens. | ||
|
|
||
| ### 2. Optimistic | ||
|
|
||
| Returns cached data immediately while fetching updates in the background. | ||
|
|
||
| ```dart | ||
| cacheConfig: StacCacheConfig( | ||
| strategy: StacCacheStrategy.optimistic, | ||
| ) | ||
| ``` | ||
|
|
||
| **Behavior:** | ||
| - ✅ Returns cached data instantly (even if expired) | ||
| - ✅ Returns cached data instantly | ||
| - ✅ Fetches fresh data in background | ||
| - ✅ Updates cache for next load | ||
| - ⚡ Fastest perceived loading | ||
|
|
||
| **Best for:** UI layouts, content screens, any screen where instant loading matters more than showing the absolute latest data. | ||
|
|
||
| ### 2. Cache First | ||
| <Warning> | ||
| With optimistic caching, users see outdated content until the next app launch after you deploy updates. Consider using `networkFirst` if immediate updates are important. | ||
| </Warning> |
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.
Clarify optimistic warning timing to avoid over-promising “next app launch.”
The warning implies updates only appear after an app relaunch, but the cache should update on the next load after the background refresh completes. Consider tightening the wording.
📝 Suggested wording tweak
- With optimistic caching, users see outdated content until the next app launch after you deploy updates. Consider using `networkFirst` if immediate updates are important.
+ With optimistic caching, users may see outdated content until the next time the screen is loaded after a background refresh completes. Consider using `networkFirst` if immediate updates are important.🤖 Prompt for AI Agents
In `@docs/concepts/caching.mdx` around lines 35 - 73, Update the Optimistic
caching warning copy to avoid implying updates only appear after an app
relaunch; instead state that cached data is returned immediately and the
background refresh will update the cache so fresh content will be visible on the
next load once the refresh completes. Edit the Warning block under the
StacCacheConfig / StacCacheStrategy.optimistic example to replace the current
“until the next app launch” phrasing with language indicating the cache is
refreshed in the background and the updated content will be shown on the next
view/load after the refresh finishes.
Description
This PR cleans up how caching works in Stac by moving everything to one place and making the default behavior more predictable.
Instead of configuring cache in multiple spots, caching is now set globally during
Stac.initialize, and allStacwidgets andStacCloudcalls follow the same rules by default.What’s inside
Stac.initializenetworkFirstis the new default strategy so users always get fresh data when onlineWhy this change
Caching logic was scattered and sometimes confusing. This makes it:
Migration notes
networkFirstType of Change
Summary by CodeRabbit
Breaking Changes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.