Preserve reactive query metadata - #6711
Conversation
🦋 Changeset detectedLatest commit: d8960f2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 29 packages
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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughReactive query factories in ChangesReactive query metadata
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/effect/test/reactivity/AtomRpc.test.ts (1)
41-55: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCover the non-finite TTL/keep-alive path in both regression tests.
Both tests currently cover only finite TTL. Add a case that verifies
Atom.keepAlivepreserves serialization after reactivity wrapping.
- packages/effect/test/reactivity/AtomRpc.test.ts#L41-L55: add non-finite TTL coverage for RPC queries.
- packages/effect/test/reactivity/AtomHttpApi.test.ts#L51-L65: add equivalent coverage for HTTP queries.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/effect/test/reactivity/AtomRpc.test.ts` around lines 41 - 55, The RPC regression test around Atom.keepAlive in packages/effect/test/reactivity/AtomRpc.test.ts lines 41-55 must also cover a non-finite TTL and verify serialization remains true after reactivity wrapping. Add the equivalent non-finite TTL/keep-alive case to packages/effect/test/reactivity/AtomHttpApi.test.ts lines 51-65; preserve the existing finite-TTL assertions in both tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/effect/test/reactivity/AtomRpc.test.ts`:
- Around line 41-55: The RPC regression test around Atom.keepAlive in
packages/effect/test/reactivity/AtomRpc.test.ts lines 41-55 must also cover a
non-finite TTL and verify serialization remains true after reactivity wrapping.
Add the equivalent non-finite TTL/keep-alive case to
packages/effect/test/reactivity/AtomHttpApi.test.ts lines 51-65; preserve the
existing finite-TTL assertions in both tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 83005aba-e719-4657-8f51-57439e8873fe
📒 Files selected for processing (5)
.changeset/fix-reactive-query-metadata.mdpackages/effect/src/unstable/reactivity/AtomHttpApi.tspackages/effect/src/unstable/reactivity/AtomRpc.tspackages/effect/test/reactivity/AtomHttpApi.test.tspackages/effect/test/reactivity/AtomRpc.test.ts
|
Addressed the keep-alive coverage suggestion in d8960f2. Both Validation:
|
Summary
AtomRpcandAtomHttpApiquery reactivity before serialization and retention metadataserializationKey, finitetimeToLive, andkeepAlivebehavior on the query atom returned to callersRoot cause
Both query constructors attached serialization and retention metadata to the base atom and then returned a new atom from
withReactivity.withReactivityis implemented as a transform, so the returned atom intentionally starts with fresh metadata. EnablingreactivityKeystherefore changed an explicitly serializable query with a one-minute TTL into a non-serializable atom withidleTTLset to0.The query constructors own the complete composition pipeline, so this change applies the behavioral transform first and then attaches serialization and retention to the final public atom. It does not change the generic semantics of
Atom.transform,withReactivity, hydration, or the registry.Red-before / green-after proof
With the regression changes applied to unmodified
main, both focused tests failed with the same result:After reordering the query composition, both tests pass and continue to verify request execution and dehydration.
Validation
pnpm lint-fixpnpm --filter effect test --run test/reactivity/AtomRpc.test.ts test/reactivity/AtomHttpApi.test.tspnpm checkpnpm lintpnpm --filter effect buildSummary by CodeRabbit