-
-
Notifications
You must be signed in to change notification settings - Fork 8.9k
feat(watch): add support for AbortController
#13861
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds AbortSignal-based cancellation to reactivity watchers. The watch API now accepts an optional signal to stop a watcher on abort. Runtime-core exposes a BaseWatchEffectOptions with signal and updates watchPostEffect/watchSyncEffect signatures. Tests verify abort-driven cancellation for watch, watchEffect, and multiple watches sharing one signal. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Caller
participant AC as AbortController
participant W as watch()
participant E as Reactive Effect
participant H as WatchHandle
C->>AC: const { signal } = new AbortController()
C->>W: watch(source, cb, { signal })
W->>E: create reactive effect
W-->>H: return handle (stop)
Note over E,H: Normal operation
E-->>C: on source change -> invoke cb
Note over AC,H: Abort path (new)
C->>AC: controller.abort()
AC-->>W: signal "abort" event
W->>H: H.stop()
H-->>E: teardown effect (unsubscribe)
Note over E: After abort: no further cb invocations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Flag potential breaking changes that are not documented:
1. Identify changes to public APIs/exports, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints (including removed/renamed items and changes to types, required params, return values, defaults, or behavior).
2. Ignore purely internal/private changes (e.g., code not exported from package entry points or marked internal).
3. Verify documentation exists: a "Breaking Change" section in the PR description and updates to CHANGELOG.md. Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
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: 2
🧹 Nitpick comments (3)
packages/reactivity/src/watch.ts (1)
126-126
: Handle pre-aborted signals and remove abort listener on manual stop
- If
signal.aborted
is already true, stop immediately.- When
stop()
is called manually, remove the abort listener to avoid retaining references.Apply:
const watchHandle: WatchHandle = () => { - effect.stop() + // detach abort listener if present + if (signal && 'removeEventListener' in signal) { + signal.removeEventListener('abort', watchHandle) + } + effect.stop() if (scope && scope.active) { remove(scope.effects, effect) } } if (signal) { - signal.addEventListener('abort', watchHandle, { once: true }) + if (signal.aborted) { + watchHandle() + } else { + signal.addEventListener('abort', watchHandle, { once: true }) + } }Also applies to: 214-221, 230-233
packages/reactivity/__tests__/watch.spec.ts (1)
293-312
: Add a test for pre-aborted signalsVerify that a watcher with an already-aborted signal never runs and detaches immediately. This guards the new fast-path.
Example:
it('stop multiple watches by abort controller', async () => { ... }) + +it('pre-aborted signal stops watch immediately', async () => { + const controller = new AbortController() + controller.abort() + const state = ref(0) + const cb = vi.fn() + watch(state, cb, { signal: controller.signal, immediate: true }) + await nextTick() + expect(cb).not.toHaveBeenCalled() + state.value++ + await nextTick() + expect(cb).not.toHaveBeenCalled() +})packages/runtime-core/__tests__/apiWatch.spec.ts (1)
435-475
: Good coverage for abort-driven stopBoth effect and source variants are validated. Consider mirroring the pre-aborted case here as well for completeness, but not required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/reactivity/__tests__/watch.spec.ts
(1 hunks)packages/reactivity/src/watch.ts
(3 hunks)packages/runtime-core/__tests__/apiWatch.spec.ts
(1 hunks)packages/runtime-core/src/apiWatch.ts
(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/reactivity/__tests__/watch.spec.ts (3)
packages/reactivity/src/watch.ts (1)
watch
(121-334)packages/runtime-core/src/apiWatch.ts (1)
watch
(130-143)packages/runtime-core/src/scheduler.ts (1)
nextTick
(61-67)
packages/runtime-core/src/apiWatch.ts (2)
packages/reactivity/src/effect.ts (1)
DebuggerOptions
(23-26)packages/runtime-core/src/index.ts (2)
DebuggerOptions
(222-222)WatchEffectOptions
(232-232)
packages/runtime-core/__tests__/apiWatch.spec.ts (2)
packages/runtime-core/src/apiWatch.ts (2)
watchEffect
(59-64)watch
(130-143)packages/reactivity/src/watch.ts (1)
watch
(121-334)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
🔇 Additional comments (1)
packages/runtime-core/src/apiWatch.ts (1)
68-69
: LGTM — signatures widened correctly
watchPostEffect
/watchSyncEffect
now accept the same base options (includingsignal
) without exposingflush
in their types. This matches usage and keeps the API tight.Also applies to: 79-80
export interface WatchOptions<Immediate = boolean> extends DebuggerOptions { | ||
signal?: AbortSignal | ||
immediate?: Immediate |
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.
🛠️ Refactor suggestion
Avoid DOM-only typing in @vue/reactivity to prevent TS breakage in non-DOM builds
Referencing AbortSignal
directly forces consumers to include lib.dom
. Reactivity is often used in Node/SSR without DOM libs, so this becomes a breaking type change. Use a structural "AbortSignal-like" type instead.
Apply:
+export interface WatchAbortSignal {
+ readonly aborted: boolean
+ addEventListener(
+ type: 'abort',
+ listener: () => void,
+ options?: { once?: boolean } | boolean
+ ): void
+ removeEventListener?(
+ type: 'abort',
+ listener: () => void
+ ): void
+}
export interface WatchOptions<Immediate = boolean> extends DebuggerOptions {
- signal?: AbortSignal
+ signal?: WatchAbortSignal
immediate?: Immediate
deep?: boolean | number
once?: boolean
scheduler?: WatchScheduler
onWarn?: (msg: string, ...args: any[]) => void
🤖 Prompt for AI Agents
In packages/reactivity/src/watch.ts around lines 49 to 51, referencing the DOM
AbortSignal type in WatchOptions forces consumers to pull in lib.dom; replace
the DOM-specific type with a minimal structural "AbortSignal-like" interface
(e.g. an exported interface with the properties/methods you actually rely on
such as readonly aborted?: boolean and
addEventListener/removeEventListener/abort optional signatures) and use that
type for signal?: instead of AbortSignal; add the interface in this file (or a
shared types file) and export it if needed so non-DOM builds won’t require
lib.dom.
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.
AbortController
and AbortSignal
was added in v15.0.0, v14.17.0 in Node.js.
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
export interface BaseWatchEffectOptions extends DebuggerOptions { | ||
signal?: AbortSignal | ||
} | ||
|
||
export interface WatchEffectOptions extends BaseWatchEffectOptions { | ||
flush?: 'pre' | 'post' | 'sync' |
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.
🛠️ Refactor suggestion
Reuse reactivity’s signal type to avoid hard DOM dependency here too
Tie the options’ signal
type to @vue/reactivity
’s WatchOptions['signal']
so both packages stay in lockstep and avoid lib.dom
requirements.
Apply:
-export interface BaseWatchEffectOptions extends DebuggerOptions {
- signal?: AbortSignal
-}
+export interface BaseWatchEffectOptions extends DebuggerOptions {
+ signal?: NonNullable<BaseWatchOptions['signal']>
+}
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/runtime-core/src/apiWatch.ts around lines 44 to 49, the
BaseWatchEffectOptions currently uses the DOM AbortSignal type; replace that
with the reactivity package's signal type by importing the WatchOptions type
from '@vue/reactivity' (import as a type-only import) and change the signal
property to use WatchOptions['signal'] so the two packages stay in sync and
avoid depending on lib.dom.
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
Can you do the same for effectScope? Or perhaps the implementation of effectScope is enough and watch doesn't need this. |
We should probably benchmark this. It would be good to have this for const {signal} = getCurrentScope()
fetch(url, {signal}) |
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.
I personally would love to see it landed!
AbortController
to watch
AbortController
That looks awesome! I’ve opened a new PR for Thank you all for the reviews and suggestions! 💚 |
This PR adds support for a new option
signal
to thewatch
API in@vue/reactivity
.It accepts an
AbortSignal
. When provided, the watcher will be stopped once the correspondingAbortController
is aborted.This provides a more flexible way to stop multiple watchers at once by sharing a single
AbortController
.Before:
After:
Summary by CodeRabbit
New Features
Tests