Skip to content

feat: make read, submit and remove awaitable - #110

Merged
alessandrobellesia merged 5 commits into
developfrom
feat/awaitable-actions
Sep 30, 2026
Merged

alessandrobellesia merged 5 commits into
developfrom
feat/awaitable-actions

Conversation

@alessandrobellesia

@alessandrobellesia alessandrobellesia commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

Problem

The obvious code looks correct and silently is not:

const { isSuccess } = await remove({ id })
if (isSuccess) { showSuccessAlert(); reload() }

The returned object was not thenable, so await returned it at once, before the DELETE ended, and isSuccess was a ComputedRef, always truthy. The success branch ran before the request finished, and also when it failed. TypeScript did not flag it. A real app on 2.0.8 had 17 such calls, all showing success alerts on failed deletes.

What changes

  • read(), submit() and remove() return a PromiseLike (like VueUse's useFetch). Awaiting it settles on the execution started by the call and resolves to the same plain snapshot execute() resolves to: booleans and values, not refs. await remove(...) types as that snapshot in the emitted .d.ts.
  • The reactive fields, execute(), keepAlive and the providers keep working as before.

Design decisions

  • immediate: false resolves at once to the current snapshot instead of typing that overload as not awaitable. TypeScript accepts await on any value, so a "not awaitable" type would not stop the code, it would bring the bug back (refs, truthy isSuccess). immediate can also be a runtime boolean, and executeWhen false at call time is not expressible in types. With the snapshot, isSuccess is false, so the wrong usage fails safe. No request is sent. The same rule applies when executeWhen is false at call time.
  • Settles on the call's own execution only. With autoExecute, later executions started by a params change do not change the result: await execute() waits for one of those.
  • Never rejects, like execute(): a failure resolves with isError: true, an aborted request with aborted: true.
  • Thenable assimilation is safe. then never starts a request and reuses one promise, so Promise.resolve(), Promise.all() or return from an async function run the action once. The snapshot is not thenable, and the store state never holds the awaitable (tested). Only then is defined, never catch: Vue's isPromise needs both, so setup() { return read() } does not become an async setup. Pinia's wrapAction and vitest spies check instanceof Promise, so the object passes through unchanged.
  • Provider template refs are not thenable, otherwise an async function returning the component instance would resolve to the snapshot.

Fixes found while making the guarantees hold

  • A read() joining an in-flight request with the same params rejected (unhandled rejection) when that request failed, and its query never showed the error. It now resolves with the error, keeps the shared error, and reports an aborted shared request as aborted: true.
  • A request that could not be built (a RepositoryHttp template with a missing path parameter) made execute() reject and left the query idle. The query now gets the error.
  • Actions called outside a component setup() (event handlers, which is exactly the pattern above) were never cleaned up, so their query, hash and items stayed in the store for ever. Cleanup now follows the effect scope (tryOnScopeDispose): components behave as before, and effectScope() and Pinia setup stores are covered too. An action awaited with no owner releases its query once the awaited execution settles, unless keepAlive. A non-awaited ownerless action keeps today's behavior (call cleanup()), so code that keeps its refs alive outside components is not broken.

Compatibility note

Released as a minor, but one case changes behavior: code that awaited an action (or returned it from an async function) and then used its refs now gets plain values. data.value after the await becomes a type error; destructuring after a top-level await in <script setup> renders once and no longer updates, with no error. The CHANGELOG ("Changed") and the README "Await" section document the migration: drop the await, or keep the returned object and await it on its own.

Docs

README "Await" section, skill references (read.md, submit.md, remove.md) and SKILL.md, CHANGELOG entry [2.1.0]. The first commit also replaces the em and en dashes in the skill files (mechanical, 59 lines).

Review

An independent read-only review round (a separate model, one round) produced 8 findings, each verified against the code:

  • Adopted:
    • sync throws when building a request (fixed, tested);
    • compatibility note in the CHANGELOG;
    • the join reads the hash again after the await (it may have been replaced);
    • the join reports an aborted shared request;
    • caveat that @volverjs/data 2.0.x reports a read() network error as aborted (only read: create, update and remove rethrow);
    • a comment that protects the PromiseLike-only choice;
    • stronger store-state checks in the tests.
  • Rejected:
    • A non-enumerable then: it would not protect defineExpose(read()) (property access still finds it), and it would make { ...read() } silently non-awaitable. Documented the defineExpose caveat instead.
    • An exported alias for the snapshot type: new public API, better as its own change.

Checks

  • pnpm test: 8 files, 81/81 passed
  • CI=true pnpm lint: clean (full rule set, not the reduced editor mode)
  • pnpm type-check: clean
  • pnpm build: clean. A consumer file compiled against dist/index.d.ts confirms await remove(...) types as the plain snapshot.
  • Each new test was checked against the develop source or a targeted mutation: they fail without the change. The provider tests pass on develop by construction and guard against regressions.

SonarCloud

  • S7739 ("Do not add then to an object") failed the gate on reliability. The thenable is intentional, and the rule exempts a then member of a class that declares implements PromiseLike, so then now lives in such a class instead of being suppressed. It is still an own arrow function field, and the public types are unchanged.
  • S5906 (23 minor): the new tests use toHaveLength() and toBeUndefined(). Existing tests are left as they are.

Not in this PR

  • Re-executing a query can abort a request that belongs to another query (a read() that joined it, or a submit()/remove() with the same params): tracked in Re-executing a query can abort a request that belongs to another query #111. With this PR the victim at least resolves with aborted: true.
  • CI runs build and tests only, so the expectTypeOf assertions in the tests are checked by pnpm type-check locally, not in CI.
  • The CHANGELOG date (2026-09-30) should be set at release time.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • read(), submit(), and remove() can now be awaited to get a plain snapshot of the execution, including success, error, or abort status. Awaiting an action that starts no request returns its current snapshot immediately.
    • Actions created without an active effect scope are cleaned up after an awaited execution settles. Set keepAlive to retain them.
  • Bug Fixes
    • Request-construction errors now appear in the action’s error state without causing the operation to reject. Reads that share failed or aborted requests also report the correct status.
  • Documentation
    • Updated guides and references with awaiting, cleanup, and version-specific usage details.

Alessandro Bellesia and others added 2 commits September 30, 2026 16:35
Use a colon, a comma or a new sentence instead, matching the house style
for prose. Mechanical change: 59 lines, no wording changes otherwise.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`const { isSuccess } = await remove({ id })` returned the reactive object
at once, before the request ended, and `isSuccess` was a ComputedRef,
always truthy, so the success branch also ran on failed requests.

The value returned by the three actions is now a PromiseLike, like
VueUse's useFetch. Awaiting it settles on the execution started by the
call and resolves to the plain snapshot `execute()` resolves to. It never
rejects, resolves at once to the current snapshot when the call starts no
execution (`immediate: false`, or `executeWhen` false), and never starts
a request itself, so thenable assimilation cannot run an action twice.
The template refs of the provider components stay non thenable.

Fixes found while making those guarantees hold:
- a read joining an in-flight request with the same params rejected when
  that request failed, lost the shared error, and reported an aborted
  shared request as not aborted;
- a request that could not be built (a missing path parameter) made
  `execute()` reject and left the query idle;
- actions are now cleaned up when their effect scope is disposed, and an
  action awaited outside any scope (an event handler) releases its query
  once it settles, unless `keepAlive`; such queries used to stay in the
  store for ever.

Code that awaited an action and then used its refs now gets plain
values: see the migration note in the CHANGELOG.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 42 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 52d55136-6838-4d11-91b3-b31f8f4bc15c

📥 Commits

Reviewing files that changed from the base of the PR and between 32672d8 and da88b22.

📒 Files selected for processing (8)
  • src/utilities.ts
  • test/error-handling.test.ts
  • test/read-advanced.test.ts
  • test/read.test.ts
  • test/remove-advanced.test.ts
  • test/remove.test.ts
  • test/submit-advanced.test.ts
  • test/submit.test.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: ca0f2a5e-cfcf-4fba-952f-0551a2498900

📥 Commits

Reviewing files that changed from the base of the PR and between 10d6ef0 and 32672d8.

📒 Files selected for processing (18)
  • CHANGELOG.md
  • README.md
  • skills/volverjs-query-vue/README.md
  • skills/volverjs-query-vue/SKILL.md
  • skills/volverjs-query-vue/references/read.md
  • skills/volverjs-query-vue/references/remove.md
  • skills/volverjs-query-vue/references/store-setup.md
  • skills/volverjs-query-vue/references/submit.md
  • src/index.ts
  • src/utilities.ts
  • test/error-handling.test.ts
  • test/read-advanced.test.ts
  • test/read.test.ts
  • test/remove-advanced.test.ts
  • test/remove.test.ts
  • test/submit-advanced.test.ts
  • test/submit.test.ts
  • test/utils.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

read(), submit(), and remove() now return awaitable action objects that resolve to execution snapshots. The changes also update request-error handling and effect-scope cleanup. Tests and documentation cover execution outcomes, ownership, and provider behavior.

Changes

Awaitable action results

Layer / File(s) Summary
Capture executions and return awaitable snapshots
src/index.ts, src/utilities.ts
The action methods retain execution promises and wrap their results as awaitable snapshots. Cleanup uses effect-scope disposal. Request-construction errors update hash error state, and shared reads preserve error or abort outcomes.
Validate await results and lifecycle behavior
test/*
Tests cover action snapshots for success, failure, abort, and no execution; query cleanup; Promise assimilation; provider instances; and request-construction failures.
Document await behavior and cleanup
CHANGELOG.md, README.md, skills/volverjs-query-vue/*
Documentation describes await results, cleanup, and keepAlive behavior. The skill references also include punctuation and wording updates.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant Action as read(), submit(), or remove()
  participant Execution as execution handler
  participant Request as request construction
  participant Awaitable as toAwaitable
  Caller->>Action: create action
  Action->>Execution: retain initial execution promise
  Execution->>Request: construct request
  Action->>Awaitable: provide execution and snapshot
  Caller->>Awaitable: await action
  Awaitable-->>Caller: execution snapshot
Loading

Merge Risk: ⚪ Minimal · up to 32672

No actionable merge-blocking issue is identified. Awaiting actions now returns plain execution snapshots rather than reactive refs, so consumers relying on refs after awaiting must adopt the documented compatibility change.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 32672

Overlapping executions created outside an effect scope can release the live query before its request finishes, losing request tracking and the eventual result. The inspected request paths retain their existing authority, but underlying authentication and transport controls were not evaluated.

Retained concerns

  • Medium · reliability · inferred: For an ownerless action with keepAlive disabled, a replacement execute() can start before the initial execution settles. Awaiting the initial execution then disables the current query, including the replacement execution. Cleanup during that request can delete its query and hash without cancellation, losing its control handles and completion status while the remote operation continues. The new automatic settlement cleanup therefore does not preserve execution ownership across overlapping transitions.
Security review details

Security Blast Radius

  • inferred — The demonstrated lifecycle exposure concerns the caller's repository store, query tracking, cached items, and requests already initiated through its configured repository. The inspected changes do not establish additional tenant, service, credential, or environment authority; downstream exposure depends on repository implementations that were not inspected.

Trust Boundaries and Controls

  • observed — Awaitable assimilation only chains an existing execution or resolves a snapshot; it does not call the repository. Parameters and repository options still cross the same inspected repository call boundary. This establishes wrapper-level preservation, not verification of authentication or transport controls inside the adapter.

Resilience and Maintainability Implications

  • inferred — The overlapping-execution concern affects ownership and failure containment: eviction can remove the live request's stored cancellation handle without stopping its remote operation. An active owning scope or keepAlive prevents the new settlement cleanup; a request that finishes before eviction also avoids this failure sequence.

Hardening Proposals

  • proposed — Bind automatic release to execution ownership or active-execution accounting so settlement of an older execution cannot evict a still-running replacement. Define explicitly whether release waits for outstanding work or cancels it before dropping tracking.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 10 files. (8 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: making read, submit, and remove awaitable.
Full details: Docstring Coverage

Explanation

Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 10 files. (8 skipped: 8 unsupported.)

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

A rabbit awaits by the clover patch,
While read and submit return a snapshot to catch.
If errors or aborts hop into view,
The result still settles, clear and true.
Scope leaves, cleanup follows the trail,
And keepAlive keeps the query hale.

Comment @coderabbitai help to get the list of available commands.

Alessandro Bellesia and others added 2 commits September 30, 2026 16:50
SonarCloud rule S7739 flags any object with a `then` property, which
failed the quality gate on reliability. The awaitable is intentional, and
the rule exempts a `then` member of a class that declares
`implements PromiseLike`: move `then` into such a class so the intent is
stated in the code instead of suppressed. `then` stays an own arrow
function field, so destructuring, spreading and reactive proxies keep
working, and the public types are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Prefer `toHaveLength()` and `toBeUndefined()` over generic `toBe()`
assertions for clearer failure reports (SonarCloud S5906). Only the lines
added by this branch; existing tests are left as they are.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
S7739 exempts a `then` field declared by a class that implements
PromiseLike, but still flagged the `this.then = ...` assignment in the
constructor. Initialize the field directly and keep its state in `#`
private fields, which stay out of the object the action returns.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@alessandrobellesia
alessandrobellesia merged commit 1a53b1b into develop Sep 30, 2026
9 checks passed
@alessandrobellesia
alessandrobellesia deleted the feat/awaitable-actions branch September 30, 2026 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant