Repository navigation
feat(Async): allowStale true/false - #8537
Conversation
🦋 Changeset detectedLatest commit: d8356e6 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
@qwik.dev/core
@qwik.dev/router
eslint-plugin-qwik
create-qwik
@qwik.dev/optimizer
commit: |
There was a problem hiding this comment.
Pull request overview
This PR introduces an allowStale option for AsyncSignal so consumers can opt out of returning the last resolved value during recomputation and instead suspend (throw the computation promise) after invalidation—useful for cases like navigation where showing stale data is undesirable.
Changes:
- Add
allowStale?: booleantoAsyncSignalOptions(defaulttrue) and implementCLEAR_ON_INVALIDATEbehavior inAsyncSignalImpl. - Add unit tests covering
allowStalebehavior and promise-throwing behavior when retrying after an error. - Update generated API/docs artifacts and add a changeset entry.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
packages/qwik/src/core/reactive-primitives/types.ts |
Adds allowStale option to AsyncSignalOptions and introduces AsyncSignalFlags.CLEAR_ON_INVALIDATE. |
packages/qwik/src/core/reactive-primitives/impl/async-signal-impl.ts |
Implements allowStale: false via a new flag that clears the cached value on invalidation; adjusts error-path behavior to reset value for retry suspension. |
packages/qwik/src/core/reactive-primitives/impl/async-signal.unit.tsx |
Adds tests for allowStale semantics and for suspending with a retried promise after an error. |
packages/qwik/src/core/qwik.core.api.md |
Updates the API extractor report to include allowStale in AsyncSignalOptions. |
packages/docs/src/routes/api/qwik/index.mdx |
Updates generated API docs to document allowStale. |
packages/docs/src/routes/api/qwik/api.json |
Updates generated API JSON content for AsyncSignalOptions to include allowStale. |
.changeset/async-allow-stale.md |
Adds a changeset entry describing the new option. |
scripts/docs-size-results.json |
Updates docs size snapshot output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (options?.allowStale === false) { | ||
| if (isDev && initial !== undefined) { | ||
| throw new Error( | ||
| 'allowStale: false and initial cannot be used together. ' + | ||
| 'allowStale: false clears the value on invalidation, which conflicts with providing an initial value.' | ||
| ); |
There was a problem hiding this comment.
In the allowStale validation path, the error is thrown after the constructor has already evaluated/applied initial (including calling initial() if it’s a function). Since allowStale: false + initial is an invalid combination, consider validating and throwing before evaluating initial to avoid unexpected side effects and work in dev builds.
| // When allowStale is false (CLEAR_ON_INVALIDATE set), clear the value so reads throw | ||
| // the computation promise instead of returning stale data (useful for navigations) | ||
| if (this.$flags$ & AsyncSignalFlags.CLEAR_ON_INVALIDATE) { | ||
| this.$untrackedValue$ = NEEDS_COMPUTATION; | ||
| } |
There was a problem hiding this comment.
allowStale: false currently clears the value only when invalidate() is called. There are other paths that mark the signal invalid without calling invalidate() (e.g. interval < 0 staleness timer sets SignalFlags.INVALID directly), so those recomputations can still return a stale value while the promise is in-flight. To make allowStale consistent, consider clearing the value when entering computation (e.g. in $computeIfNeeded$ when INVALID is observed and the CLEAR_ON_INVALIDATE flag is set) or ensuring all invalidation paths route through invalidate().
| Add `allowStale` option to AsyncSignal and routeLoader$. When `false`, invalidation clears the | ||
| previous value so reads suspend instead of returning stale data. |
There was a problem hiding this comment.
This changeset claims allowStale was added to routeLoader$, but this PR only updates AsyncSignalOptions/AsyncSignalImpl. Either update the router APIs to accept/forward allowStale for route loaders, or adjust the changeset text to avoid advertising a behavior/API that isn’t actually implemented.
github-project-automation
Bot
moved this from Waiting For Review
to Done
in Qwik Development
This allows setting
allowStale: false, which clears the signal when it is invalidated.The default is
allowStale: true.This is useful when you want a routeLoader to halt navigation while it's refetching, to prevent showing stale data.