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

feat(reactivity): improve support of getter usage in reactivity APIs #7997

Merged
merged 11 commits into from
Apr 2, 2023

Conversation

yyx990803
Copy link
Member

@yyx990803 yyx990803 commented Mar 31, 2023

TL;DR

  • One API for normalizing difference sources (value / ref / getter) to values (by introducing toValue())
  • One API for normalizing different sources (value / ref / getter) to refs (by enhancing toRef())
  • Introduce MaybeRef<T> and MaybeRefOrGetter<T> types

Context

Why Getters

It is common that we need to pass state into the composable and retain reactivity. In most cases, this means converting a reactive source into a ref:

import { toRef } from 'vue'

const props = defineProps(/* ... */)

useFeature(toRef(props, 'foo'))

Currently, toRef is only used to "pluck" a single property from an object. It is also a bit inflexible - for example, if we want to convert a nested property to a ref:

useFeature(toRef(props.foo, 'bar'))

The above code has two problems:

  1. props.foo may not exist when toRef is called
  2. This cannot handle the case if props.foo is swapped to a different object.

To workaround this, we can use computed:

useFeature(computed(() => props.foo?.bar))

However, using computed is sub-optimal here. Internally, computed creates a separate effect to cache the computation. This actually is largely overhead when the getter is simply accessing properties and not performing any expensive computations.

The least expensive way to pass non-ref reactive state into a composable is by wrapping it with a getter (or "thunking" - i.e. delaying the access of the actual value until the getter is called):

useFeature(() => props.foo?.bar)

VueUse already supports this pattern extensively. This is also a bit similar to function-style signals as seen in Solid.

In addition, this pattern will be quite commonly used when using reactive props destructure:

const { foo } = defineProps<{ foo: string }>()

useFeature(() => foo)

Introducing toValue()

On the comspoable side, it is already common for composables to accept arguments that could either be a value or a ref. This can be represented by:

type MaybeRef<T> = T | Ref<T>

To also support getters, the accepted type will be:

type MaybeRefOrGetter<T> = MaybeRef<T> | (() => T)

We currently provide unref that normalizes MaybeRef<T> to T. However, we cannot make unref also unwrap getters, because it would be a breaking change. It's possible to call unref on a function value and expect to get the function back. This is relatively rare, but it is still a possible case that we cannot break.

So, we introduce a new method, toValue():

export function toValue<T>(source: MaybeRefOrGetter<T>): T {
  return isFunction(source) ? source() : unref(source)
}

This is equivalent to VueUse's resolveUnref(). The toValue() name here is chosen since it is the opposite of toRef(): the two stands for two different directions of normalization:

ref <- toRef() - ref/value/getter - toValue() -> value

Enhancing toRef()

There maybe cases where a ref is required - you just can't pass a getter. We can still use computed for this case, but as mentioned, computed is an overkill for simple getters that just access properties.

We can add new overloads to toRef() so that it can now take getters:

const ref = toRef(() => 123)
ref.value // 123

The ref created this way is readonly and just invokes the getter on every .value access.

We also mentioned that toRef() should now be considered the API for "normalizing value / ref / getter to refs":

// value -> Ref
toRef(1) // Ref<number>

// Ref -> Ref
toRef(ref(1)) // Ref<number>

// getter -> Ref
toRef(() => 1) // Ref<number>

This is equivalent to VueUse's resolveRef().

The old toRef(object, 'key') usage is still supported, but the more flexible getter syntax should be preferred:

toRef(() => object.key)

Adoption Concerns

Backport to 2.7?

These additions create discrepancy between 3.3 and 2.7 - although in theory we are no longer adding new features to 2.7, these are probably worth backporting to ensure behavior consistency of vue-demi, and VueUse which relies on vue-demi.

If it is backported to 2.7, VueUse can also replace resolveRef and resolveUnref with toRef and toValue.

@yyx990803 yyx990803 requested a review from antfu March 31, 2023 08:39
@yyx990803
Copy link
Member Author

/ecosystem-ci run

@vue-bot
Copy link
Contributor

vue-bot commented Mar 31, 2023

📝 Ran ecosystem CI: Open

suite result
naive-ui ✅ success
nuxt ❌ failure
pinia ✅ success
router ✅ success
test-utils ✅ success
vant ✅ success
vite-plugin-vue ✅ success
vitepress ✅ success
vue-macros ✅ success
vuetify ✅ success
vueuse ✅ success

@Akryum
Copy link
Member

Akryum commented Mar 31, 2023

I like this! I guess we will be able to write:

export function useQuery (variables) {
  watch(toRef(variables), (value) => {})
}

useQuery(() => props.foo)

?

@yyx990803
Copy link
Member Author

Yes - although I'd write:

export function useQuery (variables) {
  watch(() => toValue(variables), (value) => {})
}

which would be more efficient as it doesn't create an intermediate ref.

packages/reactivity/src/ref.ts Outdated Show resolved Hide resolved
packages/dts-test/ref.test-d.ts Outdated Show resolved Hide resolved
packages/reactivity/src/ref.ts Outdated Show resolved Hide resolved
packages/reactivity/src/ref.ts Outdated Show resolved Hide resolved
packages/reactivity/src/ref.ts Outdated Show resolved Hide resolved
@jods4
Copy link
Contributor

jods4 commented Mar 31, 2023

@yyx990803 why introduce yet another way to transport a single value when we already have one?
I understand you're providing new helper functions to handle all 3 cases, but do we need them and the added complexity?
Are you gonna auto "un-wrap" getters in templates?

I think 2 cases is enough, what would be wrong with an API that creates another ref-like:

const bar = toRef(() => props.foo.bar);
// Basically:
// bar = { get value() { return props.foo.bar } }

// We can use the `bar` value, just like any other ref today:
const value = unref(bar);
// or inside templates and it's automatically unwrapped.

@bencodezen
Copy link
Member

bencodezen commented Mar 31, 2023

I'm a big fan that toValue keeps a parallel consistency with toRef as opposed to unref. Definitely has a better mental model for developers.

The improved naming and enhanced DX to the pair seem like a solid step forward for the ecosystem. Great work on this! 🙌

@yyx990803
Copy link
Member Author

yyx990803 commented Apr 2, 2023

@skirtles-code great review, fixed!

@jods4 getters only happen at the boundaries between component setup code and composables. There is no need to unwrap them in templates. Always forcing refs creates inefficiency by allocating extra one-off refs and syntax fluff. Passing getters and normalizing into values at the effect call-site is more efficient, and composable authors can handle these cases by simply using toValue over unref in all cases. More importantly, this is a pattern that is already quite extensively adopted via the usage in VueUse.

@yyx990803 yyx990803 merged commit 59e8284 into main Apr 2, 2023
@yyx990803 yyx990803 deleted the mayberef branch April 2, 2023 02:17
@bigsnowballhehe
Copy link

Yes - although I'd write:

export function useQuery (variables) {
  watch(() => toValue(variables), (value) => {})
}

which would be more efficient as it doesn't create an intermediate ref.

Sorry, I may not understand correctly, but it could have worked like this?

export function useQuery (variables) {
  watch(variables, (value) => {})
 }

@jods4
Copy link
Contributor

jods4 commented Apr 3, 2023

@yyx990803 if it's already widely use then I guess that's reason enough...

I read the whole proposal again and I see in which contexts you mean to use it, I guess it makes sense.
Also, toRef(() => x.y.z) is basically creating that passthrough ref I described, so it can be used in templates (or anywhere) if you need that. 👍

Reading your comment about how typical () => foo is when foo is a deconstructed props made me realize how trappy that is.
Deconstructed foo behaves exactly like the $ref proposal, except that proposal was dropped so it has no frame of reference and really stands out now. You'll really want to hammer in the docs that it is a macro for props.foo and spell out the caveats!

Looking forward to 3.3, between props deconstruction, this and generic components it's gonna be a great release!

@darrylnoakes
Copy link

Currently, toRef(reactiveObject, 'prop') lets me update the props on the reactive object via the created ref.
Is this syntax still the only way to do this, after these changes?
Is it recommended against?

My code uses it to interface with a Pinia store. Example:

const size = toRef(store, "fontSize");

const increase = () => {
  size.value = clamp(size.value + change, min, max);
};

const decrease = () => {
  size.value = clamp(size.value - change, min, max);
};

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.

8 participants