Necessity of reactive call on Component API result
#121
Comments
|
First of all, implicit const App = {
template: `<div @click="count++">{{ count }}</div>`,
setup() {
return { count: 0 }
}
}Users coming from Vue 2 would be surprised if this doesn't work.
A correction: readonly creates a read-only, but still reactive copy of the original reactive object. It does not prevent deep reactive conversions. The correct API to use is setup() {
return {
count: 0,
data: markNonReactive([/* lots of objects */])
}
}This is consistent with Vue 2 where all
The example use case: setup() {
let scope = {
x: ref(0),
y: ref(0),
sum() { return this.x.value + this.y }
};
return scope;
}Should not be expected to work in the first place.
This, again, is simply not something Vue has ever allowed or encouraged users to do.
What you are trying to do is essentially exposing non-reactive bindings on the render context - for which I simply don't understand what the use case is.
Forcing users to unwrap refs themselves is unergonomic:
We already have this for Vue 2 and we will in fact offer official solutions for this for v3 as well. In this case the We were well aware of private fields (in fact I was dragged into the debate by Rob Eisenberg himself). The fundamental problem is with proxies (which the public instance already is). Whether or not to implicitly call ConclusionI appreciate your suggestions, but I think they are very much biased towards your deep experience with other frameworks and no prior experience with Vue 2. In our design considerations, an extremely important aspect is the continuation of the mental model carrying over from Vue 2 and minimizing the mental shift for our existing user base, which seems completely ignored in your perspectives. |
|
I'll leave this open for a bit more discussion - |
|
In my opinion, reactivity inside Vue should be opt-out, instead of opt-in. Generally, it's expected most, or all, bindings to be reactive. |
|
On a second thought, this might be plausible if we keep root-level ref unwrapping, with some minor trade-offs. I'll experiment to see its compatibility with existing tests. |
reference: vuejs/rfcs#121 BREAKING CHANGE: object returned from `setup()` are no longer implicitly passed to `reactive()`. The renderContext is the object returned by `setup()` (or a new object if no setup() is present). Before this change, it was implicitly passed to `reactive()` for ref unwrapping. But this has the side effect of unnecessary deep reactive conversion on properties that should not be made reactive (e.g. computed return values and injected non-reactive objects), and can lead to performance issues. This change removes the `reactive()` call and instead performs a shallow ref unwrapping at the render proxy level. The breaking part is when the user returns an object with a plain property from `setup()`, e.g. `return { count: 0 }`, this property will no longer trigger updates when mutated by a in-template event handler. Instead, explicit refs are required. This also means that any objects not explicitly made reactive in `setup()` will remain non-reactive. This can be desirable when exposing heavy external stateful objects on `this`.
reference: vuejs/rfcs#121 BREAKING CHANGE: object returned from `setup()` are no longer implicitly passed to `reactive()`. The renderContext is the object returned by `setup()` (or a new object if no setup() is present). Before this change, it was implicitly passed to `reactive()` for ref unwrapping. But this has the side effect of unnecessary deep reactive conversion on properties that should not be made reactive (e.g. computed return values and injected non-reactive objects), and can lead to performance issues. This change removes the `reactive()` call and instead performs a shallow ref unwrapping at the render proxy level. The breaking part is when the user returns an object with a plain property from `setup()`, e.g. `return { count: 0 }`, this property will no longer trigger updates when mutated by a in-template event handler. Instead, explicit refs are required. This also means that any objects not explicitly made reactive in `setup()` will remain non-reactive. This can be desirable when exposing heavy external stateful objects on `this`.
|
@yyx990803 Thank you for keeping this open for discussion. I agree 100% that you must keep compatibility and mindset with the existing user base. I'm having second thoughts as well. Several of my points can be handled by alpha.3, with slightly different patterns. I would like to raise the level of the discussion. Let's not focus on that single call People won't like
|
|
Just wanted to chime in on "people won't like But from those people that have used it, they find it useful.. and as a lib, Vue has to provide something for using a I'm in the same camp that it seems awkward, but I imagine I'll also change my mind once I'm writing real stuff. |
|
I see this was changed in alpha.4. Thanks, that was really quick, you're awesome ! @michaeldrotar don't get me wrong: I love it! That's what is converting me to Vue. ❤️ |
|
As of alpha.8 this was reverted. There's a fair bit of discussions here: I created this example code that I think has a surprising behavior: |
|
@jods4 I don't think the behavior is surprising: you should make reactive any property used in your const standard = readonly([
new Color('#ff0000', 'red'),
new Color('#00ff00', 'blue'),
new Color('#0000ff', 'green'),
]) |
|
@posva I disagree here.
|
Real world case studyToday at work, we encountered this in the very first page of a new app we are working on. He was doing a basic selectable, filterable list of items. setup() {
const items = [ { ... }, { ... }, { ... } ];
return {
selected: null,
items,
}
}<ul>
<li v-for='item of items'
:class='{ active: selected === item }'
@click='selected = item'>{{ item.name }}</li>
</ul>Notice how we have Next step, he added search. Seemed easy enough. <input v-model='search' />setup() {
const data = [ ... ]
const state = {
search: '',
selected: null,
items: computed(() => data.filter(x => x.name.includes(state.search))),
}
return state
}Looks great! And search works fine.
@posva Notice how in this case there's no access to a non-ref value exposed by setup. |
|
Sidenote: I wrote a lengthy reply this morning but appearantly i forgot to press "Comment" or something, so here I go again ... :/
This is correct, and the underlying culprit. And this culprit is not directly linked to wether or not we make To demonstrate, this is a variation of your example, as a reusable composition function, returning reactive state, as you might write them whenever you extract reusable code from a component: function useMyFeature() {
const data = [ ... ]
const state = reactive({
search: '',
selected: null,
items: computed(() => data.filter(x => x.name.includes(state.search))),
})
// further implementation
return state
}This code will have the practically the same problem. It's a little more apparent since you actually *see that So the underlying issue is: How can e keep people from accidentally comparing raw values (objects) against proxys of those values, which leads to failing comparisons? |
|
@LinusBorg you are correct. It was so strikingly similar to the previous example that I didn't think it through. The root cause is subtly different. It's not wrapping with So let's put that aside. My previous example from this comment: #121 (comment) |
|
I think it's actually the same thing, the issue is that a proxy object is compared to an array of non-proxy objects, so no match is found. It seems to me that you got a little side-tracked by the fact that posva proposed to use a Check out this version which works fine after wrapping the array in https://codesandbox.io/s/wonderful-smoke-j8ckx I understand that this can be counterintuitive for some, as you don't want to wrap your static arrays in a https://codesandbox.io/s/stoic-bell-docej And just to repeat: We have to find a general strategy to keep people from stepping into this trap, I think we agree on that. E.g. we have to make it clear that:
Relating to 1., I think if we reverted to alpha.7 behaviour, we would actually have some proxies and some raw values going back and forth, which people would have to track in order to not produce a |
|
@LinusBorg I was writing an answer then thought some more and scrapped it. My opinion right now is that for anything but trivial cases you need to understand how this stuff works (what is (un-)wrapped automatically, where are the boundaries). IMHO the less magic, the better because you aren't surprised by hidden behaviors. That's why I preferred alpha 7. I know this means deep refs aren't automatically unwrapped in templates, but you can make them unwrap by placing Because no solution is strictly better, I'm personally happy with alpha 8, as long as I get those two little functions that I asked for here: My default coding style is gonna be to return state with a shallow unwrapping proxy (i.e. like alpha 7) and rename the primitives to be shallow by default. That's enough 80% of the time, more performant and it leads to very predictable behavior. It's cool that Vue gives me this flexibility to choose the coding style I like. 👍 About the 1st exampleThe point in this example is that an array of well known color names is not supposed to change. Therefore I would like to to freeze it, or call The solution is then to use a setup() {
const standard = markNonReactive([ Color, Color, Color ])
return {
standard,
custom: new Color('#0e88e0'),
selection: shallowRef(standard[0]),
isStandard(x) {
return standard.includes(x)
},
}
} |
Yes, I agree with you and I have one. TL;DR; Proxy must happen at creation time. The problem when using proxies is that you lose referential identity. You have 2 different references for the same instance. If you mix them up (which in a complex app is inevitable), we know that stuff breaks. The solution is to avoid those problems by not having 2 references for a single object. The easiest way to ensure that is to create proxies when you create those objects, never later, and throw away the raw reference. As it stands, Vue 3 encourages the late creation of proxies.
This is very much a opt-in vs opt-out choice and Vue is currently encouraging opt-out.
You can stop reading here if this comment is too long, the rest is me commenting on those points 😉 One example in both stylesLet's say I'm fetching a list of 600 items from the server and displaying them in a virtual list that enables selection of items. The list component has a reactive Because it's 600 immutable objects, I don't want them to be reactive to (1) indicate intention; and (2) perf. Here's the code in proxy at creation philosophy: // Note: I swapped the shallow/deep implementations
import { reactive as deepReactive, shallowReactive as reactive, shallowRef as ref } from 'vue'
setup() {
const items = ref(null) // shallow
const selection = reactive(new Set()) // shallow
const count = computed(() => selection.size) // Fun fact: computed is already shallow
// Get the data from server
fetch('/data').then(r => r.json())
.then(data => {
// data is an immutable array, I don't want to wrap them:
items.value = data
// on the other hand, if I wanted to have editable items, I would have done this:
// items.value = deepReactive(data)
// in both cases, there's only 1 reference of data when returning
// and _no code in my component_ will create new proxies
})
// Note: hideRefs could/should be done by Vue and not part of the real code
return hideRefs({
selection,
count,
items,
})
}Bonus: only what really needs to be reactive is, as this is an explicit choice, so better perf. In typical Vue 3 style, it would all mostly be the same, but making data non-reactive is tricky. items.value = markNonReactive(data)But this is not deeply non-reactive. As soon as one item is taken out of Final wordsI can't believe users will wrap everything into reactive state. True story: the example I previously gave with the computed happened the very first day a dev on my team tried to create a list with Vue 3. This is scary because it will lead to stack overflow questions, blog posts, frustration and some opinion that Vue 3 reactivity is harmful, too complicated or bad idea (I think it's a good one). I think Vue 3 should promote reactive at creation time patterns and safer defaults (shallow, no hidden calls) that lead users towards code that always works. |
|
@jods4 I appreciate your thoughts on this matter. There's interesting stuff in there, but I disagree pretty fundamentally.
What has been the default in Vue 2?For starters, the default behaviour that people know and like from Vue 2 is that of what you call The only escape hatch people had was to manually add a nonreactive property on the vm, like this: data() {
this.nonReactiveProperty = { foo: 'bar' }
return {
myReactiveList: [ /* ... */]
}
}..which came with the caveat that this property itself was not reactive, so assigning a new object later (i.e. after "deep" reactivity as a default is what makes Vue so approachable. As soon as you stick anything into What I think we both agree on is that the switch to proxys in v3 does open the door to a few tripwires as soon as you start mixing reactive state (proxies) with nonreactive objects (raw objects/arrays). However, your proposition won't sufficiently solve that problem - I think it will make it worse. To explain reasons for this opinion, I'll go through the benefits that you see in your proposition: Supposed benefits of your proposition
Let's look at them individually: Better communication of intent.In Vue 3, if you have e.g. an array that you want to be immutable, you can actually make it immutable with
Less proxies will mean better performance.In short, your are technically right here, of course. But this is only important when there is actually a performance problem to worry about that can be solved with non-reactive data. In our experience, making stuff reactive was rarely the performance bottleneck in Vue 2 - it's usually in rendering/patching, if you experience one at all. And In Vue 3, reactivity with proxies performs even better in terms of memory allocation and CPU. Also, from looking at your example, you seem to agree that if you want to edit items from a 600-item list, you would make it reactive anyway. So the performance benefits only apply to strictly immutable state, and will only be noticeable in very large lists. So I think you are essentially proposing a premature optimisation instead of optimizing when and where it is actually required, as a conscious decision. Don't get me wrong, "better performance as a default" is a great approach and basically what reactivity gives you over approaches like React's "shouldComponentUpdate", but if it's coming at thew price of DX and maintainability, we shouldn't optimize a problem we are not sure we are heaving. So: Correct, but not as relevant as you make it out to be. And without a huge relevance, its cost is too high from my perspective: Safer defaultHere is the part where we agree pretty fundamentally. the example that you demonstrated your approach with is overly simplistic and glosses over the fact that you will need (or at least want) deep reactivity in all but the most simplistic of situations:
Those are not edge cases, they are the daily bread and butter operations that any Vue dev deals with. Making them the "non-default" and thereby expecting people to work with shallow proxies and raw objects wherever possible will quickly lead to exactly those scenarios that we want to avoid - mixing proxies and non-proxies in unexpected places. Here's a quick example: export default {
setup(){
// a static list, so let's not make it reactive (or shallowly reactive, same issue)
const categories = [{ id: 'reactivity', label: 'Reactivity' }, /*...*/ ]
// we want to edit this post, so let's make it reactive
const post = deepReactive({
title: '',
body: '',
// since we also want to edit these categories, we need "deep" reactivity
categories: [],
})
}
function addCategory(category) {
post.categories.push(category)
}
const availableCategories = computed(() => {
return categories.filter(cat => post.categories.include(cat))
})
}Did you catch it? This is such a common task, and we are already mixing proxies and non-proxies. And run into the caveats that the "safe defaults" should guard people from. Unfortunately, they don't. Now lets do the same example with deep reactivity / "always use proxies" approach: export default {
setup(){
// a static list, let's actively make it readonly.
// We don't want the dev to touch it, anyway.
const categories = readonly([{ id: 'reactivity', label: 'Reactivity' }, /*...*/ ])
// we want to edit this post, so let's make it deeply reactive
const post = reactive({
title: '',
body: '',
// since we also want to edit these categories, we need "deep" reactivity
categories: [],
})
}
function addCategory(category) {
post.categories.push(category)
}
const availableCategories = computed(() => {
return categories.filter(cat => post.categories.include(cat))
})
}...and now everything works, because all objects are actually proxies. My propositionMy feeling is that we should do pretty much the opposite of what you propose: We should encourage users to wrap all of their state in one of these:
...thereby making sure that in our component we only deal with proxies. We could even provide lint rules for that This has the following benefits:
We can document how to deal with But this approach definitely also requires us to make people aware of the drawbacks and tripwires of proxies nonetheless - but the recommendation becomes: *"To keep away from possible mismatches, make everything reactive. If you experience a situation where that seems to impact performance, read chapter XX to learn how to use raw objects alongside proxies created by our reactivity system and what to watch out for." |
|
@LinusBorg Thanks for having this discussion. Your solution (always use either It requires discipline, but if the docs spell it out clearly and every example applies it consistently, it may become a natural mindset. I have a nit with The perf issue with large quantities of static data (long lists, dataviz) is not so much the proxy creation (cheaper than getters) nor the proxy overhead (slower than getters but not significant). Rather it's the cost of tracking. Tracking makes additional function calls and lots of memory allocations. On a (non micro-) benchmark I could improve perf by 50% and memory usage by 30% by cutting down tracking. That's significant.
👉 This is easily fixed by introducing a (A pitfall of Assuming everyone wraps all state in either The main argument was: setup() {
let position = useMouse() // returns { x: Ref, y: Ref }
return { position, otherStuff }
}This goes against our every state should be proxy philosophy. But I'm sure it'll be common anyway. I still believe that returning a reactive We can still support this pattern by calling |
|
I'd just like to drop my 2 cents here. Reactive wrappingI don't think Vue 2's I use Vue 2 professionally, and I do expect I can only speak for myself here but to me For Ref unwrapping in templatesBased on alpha.8, sounds like currently the primary purpose of reactive wrapping is to support automagic ref unwrapping in templates. To be honest I wasn't and still isn't sold on auto unref in templates. Since we need to use If it were up to me I'd remove both auto unref and reactive wrapping, but as a middle ground, the deep |
|
The scope has changed a lot between what I initially wrote and all the comments, changes in alphas, and discussions that happened around reactvity since then. To help finalize reactivity for the beta, I starting anew with the current state in #157 and closing this one. |
This is not a proposal per se but I would like to raise some points about always calling
reactiveon the value returned bysetup(return value that I'm gonna namescopein this issue).Code is right here:
https://github.com/vuejs/vue-next/blob/master/packages/runtime-core/src/component.ts#L355
Intro: what it achieves
Calling
reactive()on the return value ofsetup()enables:refinsetup.refvalues for easy consumption in template.1. Missed optimizations
reactive()makes every field and property deeply reactive by default.This can be counter-intuitive and misses some optimization opportunities.
Consider:
I would expect
countto be a reactive property, whiledatais a non-reactive array. Maybe some read-only objects I got from server and I don't intend on modifying.This gives me total control over the reactivity, and only what I intend on modifying is actually reactive.
But the scope will actually be fully reactive. This means every object in
datais gonna get wrapped in a proxy. Every UI binding is gonna create change listeners. All this work could have been avoided.Of course there's
readonly()that enables me to avoid that if I really want to, but the default behavior doesn't feel intuitive.It's also slightly inconsistent. Why create reactive data in
setup, if it'll be reactive anyway? Answer: you need to create reactive data if you intend to use it insidesetup, e.g. incompute()orwatch(). Otherwise you don't have to. It feels weird and arbitrary.2. To value or not value?
Speaking of inconsistency, Whether you have to use
valueor not is also a bit inconsistent.Some users may deviate just slightly from the recommended pattern and do this:
Using
thisinsumkind of works, but cannot work everywhere.Inside
setup, if you create awatchfor example,scope.sumworks if you dothis.x.value, becausethisisscopeandxis aref.In the template on the other hand,
sumworks if you dothis.ybecausethisisreactive(scope)and therefhas been unwrapped.Of course, the solution is to not use
thisbut refer toscopedirectly. You can be sure some users will fall into this trap.Using
scopedoesn't solve everything either, as we fall back into the inconsistencies of 1. If I declare something without arefinsidescope, sayx: 0then event handlers that set it onscopewill escape the change tracking, despite everything I said in 1 (since they would access the target of the proxy directly).3. Unwrap refs ourselves
Point 2 shows that
.valueusage can become confusing.History (I'm referring to Knockout here) also shows that devs don't like having accessors everywhere: they're verbose and it's easy to forget or misuse them if you don't have a type system (e.g. you write plain JS).
So you may be tempted to do this:
And now you can use
scopeall you want without ever writing a.value.This works great, but at this point doing
reactiveon the returned value is a noop.If we go back to 1, and you want to have easy, fine control over what property is reactive and which one is not, you may be tempted to write your own function, say
hideRefsthat either transform every field holding a ref into a getter/setter that unwraps them; or wrap the thing behind a proxy that does the same.In theory this is a great solution but the automatic
reactivewill uselessly wrap this into a proxy. Additionally, every access (read/write) tocountwill be double-tracked, once by thereactiveproxy, and once by therefhidden inside.4. Class-based components
I totally understand why Vue 3 is gonna stay away from decorators, at least until they advance to a further stage.
That said, some users may still think using decorators in their project is ok. They might like the following syntax:
It's not very complicated to write the decorators that make this work. It won't be long before someone publishes them and that's fine.
There are two issues here:
hideRefstechnique described in 3.As far as I can tell, if
reactivewas not called on the scope automatically, this would work perfectly.Conclusion
I love the explicit control given by
refand the new Composition API ❤️In fact, I've been wanting this in a modern framework since the day I stopped using Knockout.
I feel like the automatic
reactivecall on the scope is removing a lot of that explicit control -- or making it a lot more verbose to reclaim (readonlyand co.).First idea that comes to mind is that users should be responsible for the precise shape of their scope, i.e. they need to call
reactivethemselves if they want to.setup.reactiveanyway, because it's more convenient than doing lots ofrefandvalue.toRefs...Also: I think the
hideRefs(actual name to be defined) function I mentionned above should probably in Vue core. It's a better choice to call on the scope thanreactive.The text was updated successfully, but these errors were encountered: