fix(types): watchEffect typing does not support async callback (#14249)

[zhanghaifeng213]

close #14249

Summary by CodeRabbit

• Refactor
  • Watch effects and cleanup handlers can now be asynchronous (may return a Promise) while preserving synchronous behavior.
• Tests
  • Tests updated to reflect that effect callbacks no longer return a value and now receive the watched value when invoked.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The WatchEffect type was broadened to accept async effects (Promise<void> | void). A test in apiWatch.spec.ts was adjusted: the watchEffect callback now passes the watched value (arr) to the spy and no longer returns it.

Changes

Cohort / File(s)	Summary
Reactivity types packages/reactivity/src/watch.ts	Updated WatchEffect signature to `export type WatchEffect = (onCleanup: OnCleanup) => Promise
Tests packages/runtime-core/__tests__/apiWatch.spec.ts	Updated watchEffect test: effect now calls the spy with the watched arr and no longer returns arr (returns void).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• chore(types): improve type safety in watch functions and instanceWatch #13918 — Related changes to watch/watchEffect typings and return signatures.

Suggested labels

ready to merge, :broom: p1-chore

Poem

🐰 I nudged the types to let async play,
Effects can pause and hop away,
Tests now peek at the array in sight,
No more returns — just morning light.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(types): watchEffect typing does not support async callback' accurately and concisely describes the main change—updating WatchEffect type to support async callbacks.
Linked Issues check	✅ Passed	The PR successfully addresses issue #14249 by updating the WatchEffect type signature from void to Promise | void, matching the requested fix.
Out of Scope Changes check	✅ Passed	The test file changes in apiWatch.spec.ts appear minimal and focused on adjusting test behavior to align with the type signature update, remaining within scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bfb20cb and 73ddef8.

📒 Files selected for processing (1)

• packages/reactivity/src/watch.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/reactivity/src/watch.ts

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/reactivity/src/watch.ts (1)

37-37: Critical: The WatchEffect type was not updated to support async callbacks.

The PR objectives and issue #14249 request changing WatchEffect to allow async callbacks, but this line remains unchanged. The fix should be:

-export type WatchEffect = (onCleanup: OnCleanup) => void
+export type WatchEffect = (onCleanup: OnCleanup) => Promise<void> | void

This is the actual type that governs whether watchEffect can accept an async callback. Without this change, TypeScript will still reject async watchEffect callbacks.

🔎 Proposed fix

-export type WatchEffect = (onCleanup: OnCleanup) => void
+export type WatchEffect = (onCleanup: OnCleanup) => Promise<void> | void

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c68bebf and c3a612e.

📒 Files selected for processing (1)

• packages/reactivity/src/watch.ts

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

packages/reactivity/src/watch.ts (1)

47-47: Critical: Revert OnCleanup type change - promises would be ignored.

The change to OnCleanup allowing Promise<void> is incorrect and creates a type/implementation mismatch:

• OnCleanup is the type of the cleanup registration function (the onCleanup parameter passed TO your watchEffect callback)
• The implementation at line 292 (boundCleanup = fn => onWatcherCleanup(fn, false, effect)) calls onWatcherCleanup which returns void (line 107)
• Any promise returned by OnCleanup would be completely ignored by the implementation

Only WatchEffect (line 37) should allow async return - the cleanup registration itself must remain synchronous.

🔎 Proposed fix

-export type OnCleanup = (cleanupFn: () => void) => Promise<void> | void
+export type OnCleanup = (cleanupFn: () => void) => void

Based on past review comments.

🧹 Nitpick comments (1)

packages/runtime-core/__tests__/apiWatch.spec.ts (1)

1107-1108: Clarify: Why change spy() to spy(arr)?

The removal of return arr at line 1108 makes sense for type compliance (watchEffect callbacks should return void | Promise<void>, not a Ref). However, the change from spy() to spy(arr) at line 1107 is unclear:

• The test assertions only check call counts (toHaveBeenCalledTimes), not spy arguments
• This change doesn't affect test behavior
• It doesn't appear directly related to the type fix

Is this an intentional improvement to make the test track the watched value, or an accidental change?

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c3a612e and bfb20cb.

📒 Files selected for processing (2)

• packages/reactivity/src/watch.ts
• packages/runtime-core/__tests__/apiWatch.spec.ts

🧰 Additional context used

🧬 Code graph analysis (1)

packages/reactivity/src/watch.ts (3)

packages/runtime-core/src/index.ts (1)

• WatchEffect (230-230)

packages/runtime-core/src/apiWatch.ts (2)

• WatchEffect (29-29)
• OnCleanup (32-32)

packages/reactivity/src/index.ts (2)

• WatchEffect (93-93)
• OnCleanup (96-96)