-
-
Notifications
You must be signed in to change notification settings - Fork 8.9k
feat(types): enhanced types on @vue/reactivity
watch
#13840
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
WalkthroughRe-exports the MultiWatchSources type from @vue/reactivity, removes the local alias in runtime-core, and updates exports accordingly. In doWatch, the callback passed to baseWatch is cast to any. No runtime behavior changes are introduced; updates are limited to type exports and internal typing. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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: |
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: 0
🧹 Nitpick comments (2)
packages/reactivity/src/watch.ts (1)
327-331
: Avoid ts-expect-error by casting cb once.
Keeps types clean and prevents “expect error not found” drift across TS versions.Apply:
- call - ? call(cb!, WatchErrorCodes.WATCH_CALLBACK, args) - : // @ts-expect-error - cb!(...args) + const cbAny = cb as any + call ? call(cbAny, WatchErrorCodes.WATCH_CALLBACK, args) : cbAny(...args)packages/runtime-core/src/apiWatch.ts (1)
225-225
: Casting cb as any when calling baseWatch is acceptable here.
Prevents complex generic explosion while preserving typed overloads at the API boundary.Consider a short comment explaining why the cast is necessary to help future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/reactivity/src/index.ts
(1 hunks)packages/reactivity/src/watch.ts
(2 hunks)packages/runtime-core/src/apiWatch.ts
(2 hunks)packages/runtime-core/src/index.ts
(1 hunks)
🔥 Files not summarized due to errors (1)
- packages/reactivity/src/watch.ts: Error: Server error: no LLM provider could handle the message
🧰 Additional context used
🧬 Code graph analysis (1)
packages/reactivity/src/watch.ts (2)
packages/reactivity/src/index.ts (8)
MultiWatchSources
(89-89)WatchSource
(95-95)watch
(84-84)WatchEffect
(94-94)WatchOptions
(90-90)WatchHandle
(93-93)WatchCallback
(96-96)ReactiveMarker
(41-41)packages/runtime-core/src/apiWatch.ts (6)
WatchSource
(30-30)watch
(125-138)WatchEffect
(29-29)WatchOptions
(49-53)WatchHandle
(27-27)WatchCallback
(31-31)
⏰ 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 (6)
packages/reactivity/src/watch.ts (3)
24-24
: Importing ReactiveMarker/isReactive/isShallow is correct and used below.
No issues; aligns with overload discrimination and runtime checks.
120-131
: Type utilities mirror runtime-core and enable generic oldValue typing.
MaybeUndefined, MultiWatchSources, and MapSources look consistent and should fix the generic inference gap.
132-177
: Overload set restores full generic parity with runtime-core.
The added WatchEffect-first overload and the tuple/array/object sources overloads look right and unblock typed new/old values.packages/reactivity/src/index.ts (1)
89-89
: Re-exporting MultiWatchSources from watch is the right public surface.
Keeps a single source of truth for the alias and simplifies runtime-core usage.packages/runtime-core/src/apiWatch.ts (1)
4-4
: Import MultiWatchSources from @vue/reactivity.
Good consolidation; removes local alias duplication.packages/runtime-core/src/index.ts (1)
227-228
: Type re-export for MultiWatchSources from reactivity looks good.
Ensures stable public typing for consumers importing from 'vue'.
close #13807
I effectively reused the type signatures from
apiWatch.ts
with an additional overload forWatchEffect
as the first argumentSummary by CodeRabbit