Skip to content
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

types(effect): pass type argument to return type in effect #7664

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

CyanSalt
Copy link

@CyanSalt CyanSalt commented Feb 7, 2023

The return value of the effect function currently has an overly loose type (any). Refer to the following example:
image

And

image

In this example, effect accepts an fn which will return a Promise<void>, and since the return value of effect is actually the return value of ReactiveEffect#run, which is the return value of fn. So effect should also return a Promise<void>. However, due to the lack of type argument passing, the type is currently derived as any.

After this PR change it looks like this
image

And

image

And

image

@CyanSalt CyanSalt changed the title fix: fix the return type of effect fix(reactivity): pass type argument to return type in effect Feb 15, 2023
@skirtles-code
Copy link
Contributor

Similar to #6024.

# Conflicts:
#	packages/reactivity/src/effect.ts
@pikax pikax added scope: types 🧹 p1-chore Priority 1: this doesn't change code behavior. labels Oct 25, 2023
@github-actions
Copy link

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 86.5 kB 32.9 kB 29.7 kB
vue.global.prod.js 132 kB 49.6 kB 44.5 kB

Usages

Name Size Gzip Brotli
createApp 47.9 kB 18.9 kB 17.2 kB
createSSRApp 51.2 kB 20.2 kB 18.4 kB
defineCustomElement 50.3 kB 19.7 kB 17.9 kB
overall 61.3 kB 23.7 kB 21.6 kB

@pikax pikax changed the title fix(reactivity): pass type argument to return type in effect types(effect): pass type argument to return type in effect Oct 25, 2023
@pikax pikax added the ready to merge The PR is ready to be merged. label Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧹 p1-chore Priority 1: this doesn't change code behavior. ready to merge The PR is ready to be merged. scope: types
Projects
Status: Ready to merge
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

3 participants