vuejs / rfcs

RFCs for substantial changes / feature additions to Vue core
4.88k stars 546 forks source link

Necessity of `reactive` call on Component API result #121

Closed jods4 closed 4 years ago

jods4 commented 4 years ago

This is not a proposal per se but I would like to raise some points about always calling reactive on the value returned by setup (return value that I'm gonna name scope in 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 of setup() enables:

1. Missed optimizations

reactive() makes every field and property deeply reactive by default. This can be counter-intuitive and misses some optimization opportunities.

Consider:

const MyApp = {
  setup() {
    return {
      count: ref(0),
      data: [/* lots of objects /*],
    };
  }
};

I would expect count to be a reactive property, while data is 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 data is 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 inside setup, e.g. in compute() or watch(). 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 value or not is also a bit inconsistent.

Some users may deviate just slightly from the recommended pattern and do this:

setup() {
   let scope = {
     x: ref(0), 
     y: ref(0),
     sum() { return this.x.value + this.y }
   };
  return scope;
}

Using this in sum kind of works, but cannot work everywhere.

Inside setup, if you create a watch for example, scope.sum works if you do this.x.value, because this is scope and x is a ref. In the template on the other hand, sum works if you do this.y because this is reactive(scope) and the ref has been unwrapped.

Of course, the solution is to not use this but refer to scope directly. You can be sure some users will fall into this trap.

Using scope doesn't solve everything either, as we fall back into the inconsistencies of 1. If I declare something without a ref inside scope, say x: 0 then event handlers that set it on scope will 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 .value usage 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:

setup() {
   let scope = reactive({ x: 0, y: 0 });
}

And now you can use scope all you want without ever writing a .value. This works great, but at this point doing reactive on 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 hideRefs that 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.

setup() {
   // count is reactive, data is not, everything is usable without .value
   let scope = hideRefs({ count: ref(0), data: []);
}

In theory this is a great solution but the automatic reactive will uselessly wrap this into a proxy. Additionally, every access (read/write) to count will be double-tracked, once by the reactive proxy, and once by the ref hidden 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:

@component
class MyApp {
  @ref x = 0;
  @ref y = 0;

  @computed 
  get sum() { return this.x + this.y }

  @watch
  doSomething() { }
}

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:

As far as I can tell, if reactive was not called on the scope automatically, this would work perfectly.

Conclusion

I love the explicit control given by ref and 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 reactive call on the scope is removing a lot of that explicit control -- or making it a lot more verbose to reclaim (readonly and 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 reactive themselves if they want to.

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 than reactive.

yyx990803 commented 4 years ago

First of all, implicit reactive call allows you to do this:

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.

  1. Missed optimizations

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 markNonReactive (which is not documented in the composition API, due to potential name changes):

setup() {
  return {
    count: 0,
    data: markNonReactive([/* lots of objects */])
  }
}

This is consistent with Vue 2 where all data() returned values are reactive by default and sub trees that should not be converted are explicitly marked with Object.freeze().

  1. To value or not value?

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 would point to the plain scope only when called as scope.sum(). When scope is returned there is no guarantee of what this context would be when sum is invoked in the template. This is not inconsistency per-se, it's just how this works by design. You can't expect this to work consistently if you are not the one calling the method - this applies for anything JavaScript and isn't a result of the API design. By the way, in the docs we should recommend never using this in setup().

Fine-grained control of which property is reactive

This, again, is simply not something Vue has ever allowed or encouraged users to do.

markNonReactive (or Object.freeze) already allows you to mark any object value to skip reactive conversions. (btw this is much more intuitive and explicit than your hideRefs, in my opinion).

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.

Unwrap refs ourselves

Forcing users to unwrap refs themselves is unergonomic:

Class-based components

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 this would always be pointing to the PublicInstanceProxy so there isn't any of the problems you mentioned above.

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 reactive on setup return value is irrelevant.

Conclusion

I 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.

yyx990803 commented 4 years ago

I'll leave this open for a bit more discussion - but note it's unlikely for us to consider changing this behavior.

leopiccionia commented 4 years ago

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.

yyx990803 commented 4 years ago

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.

jods4 commented 4 years ago

@yyx990803 Thank you for keeping this open for discussion. You are 100% correct about my background. Vue 3 caught my attention because of the new reactivity primitives and Composition API. I didn't like the way Vue 2 handled state.

I agree 100% that you must keep compatibility and mindset with the existing user base. I also hope that the new API is well designed on its own, without considering v2. Hopefully you can balance the two 🤞

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 reactive() call but rather at the big picture, i.e. API usage.

People won't like value

Experience shows that it's tedious to put value everywhere and error-prone (esp. in raw JS).

ref is necessary when you want to pass reactive individual values around. No way around this.

But when working on a component, you usually think of a set of properties. Especially Vue 2 devs that are used to data. A natural pattern for them could be:

setup() {
   let data = reactive({ x: 0, y: 0 });
   data.sum = computed(() => data.x + data.y);
   return data;
}

I think it's a natural conversion and you generally won't need value anywhere.

In this situation the value returned by setup won't be wrapped by another proxy, which has the nice property that the binding scope is the same (===) as data in my code. Easy to reason about and can avoid a few edge cases.

Several ways mixin can fit in, for example:

function useThingy() {
  let publicState = reactive({ count: 0, frob: 'never' });
  let privateState = reactive({ mine: 1 });
  watch(() => 'something something');
  return toRefs(publicState);
}

setup() {
  let data = reactive({ x: 0, y: 0, ...useThingy() });
}

Flexibility

The beauty of this well layered architecture is that users can customize it to any patterns they may like! As long as deep down it uses the reactivity primitives, it should work.

I can return my non-reactive data by doing:

let scope = reactive({ 
  count: 0, 
  data: markNonReactive([,,]) 
});

But maybe I like doing it the opposite way: only what I described as reactive is. I can easily write a function that would proxy my object by unwrapping refs, no more:

let scope = unwrap({ 
  count: ref(0), 
  data: [,,]
});

In some case it might work very well and it's lightweight (it does less work than the reactive proxy). In fact, I'd probably mix-and-match in my code based on the situation.

I think it's beautiful but it's shattered by the special treatment reactive is given in alpha.3. The reactivity layer is nice and open, but Vue makes a usage that give the built-in proxies a very special status and closes extensibility.

Note that there is a subtle difference between the 2 examples: the array is non-reactive in both, but in the first case the data property itself is reactive (I could swap in another array), in the second it's not.

Magic or explicitness

Bindings are reactive most of the time. But sometimes you have a read-only scenario. E.g. a screen that shows details about an entity, the result of a search, etc.

The most optimized way to do that is to bind non-reactive data, just plain old objects.

setup() {
  return markNonReactive({ lots: '', of: '', properties: '' });
}

I'm happy this is possible and easy.

Open question: would it be less intuitive if there was no magic reactivity? If what you return is the scope you get?

setup() {
  return { lots. '', of: '', properties: '' };
}

The drawback is that the lack of magic means users have to explicitely call reactive. In the most common case! That sounds bad! But don't they already? Unless you do nothing with the reactive state, you have to use reactive or ref anyway. If you stick to plain objects none of your changes on plain objects will be picked up by bindings.

setup() {
  let data = { 
    title: 'whatev', 
    count: 0,
    inc() { data.count++ } 
  };  
  return data;
}

Example above won't work. You need to make data reactive or at least make count a ref.

I see only one way to not stick to plain objects: use this in the event handler inc(), which is gonna be the proxy... but your recommandations is to never use this.

Conclusion

What do you think? 😃

BTW feel free to close this if you think it's nonsense. I'm happy because I realized that markNonReactive can be used to opt-out of that reactive call on the scope. Although I knew what the function did, I didn't immediately realize what it would do in this context.

So opt-in / opt-out, your call!

michaeldrotar commented 4 years ago

Just wanted to chime in on "people won't like value" since the topic has already been hashed out a dozen ways -- but the short answer is you don't have to use it... if you don't like it for your stuff, just stick with reactive. There's nothing wrong with the examples you gave and there's no reason to feel pressured to use every single tool in a toolbox.

But from those people that have used it, they find it useful.. and as a lib, Vue has to provide something for using a const x = 3 type of syntax.. so const x = ref(3) is the best option so far.

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.

jods4 commented 4 years ago

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. ❤️

jods4 commented 4 years ago

As of alpha.8 this was reverted.

There's a fair bit of discussions here: https://github.com/vuejs/vue-next/commit/e67f655b2687042fcc74dc0993581405abed56de It's probably better to bring back the discussion here, that is if there's anything left to discuss.

I created this example code that I think has a surprising behavior: https://codesandbox.io/s/elegant-jepsen-ydunm

posva commented 4 years ago

@jods4 I don't think the behavior is surprising: you should make reactive any property used in your setup function that is later on returned inside it. In this case, since it doesn't change, using readonly seems to be even better:

const standard = readonly([
            new Color('#ff0000', 'red'),
            new Color('#00ff00', 'blue'),
            new Color('#0000ff', 'green'),
          ])
jods4 commented 4 years ago

@posva I disagree here.

  1. Do you expect users to wrap every single value returned by setup into a ref? Then at this point there's no need to wrap the setup object into a reactive, just a proxy that unwrap 1st level refs. Hey, that's what we had in alpha 7! 😁

  2. standard is an immutable array. How is it intuitive to wrap it into ref? If anything, I'd pass it to markNonReactive but that doesn't fix the problem. In fact, for optimization purposes I'd want to pass it to markNonReactive.

  3. Do you believe this won't be an issue? Nobody amongst the many users of Vue will have a bad experience with this? A good API is a "pit of success", it naturally leads to good/correct usage. Arbitrary rules not conveyed by API, such as "you should make reactive any property used in your setup function that is later on returned inside it" are not great API design.

jods4 commented 4 years ago

Real world case study

Today at work, we encountered this in the very first page of a new app we are working on. Genuinely not trying to break anything. The junior dev was stuck without a clue what was going on. I immediately had a clue but it still took me a bit to figure it out fully.

He was doing a basic selectable, filterable list of items. He started with just a selectable list. It was very easy and worked well:

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 selected === item and selected = item next to each other. It seems so right. Hard to see what's gonna go wrong, right?

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. But we broke selection. Try to figure out for yourself why, I'm gonna spoil the answer.

computed doesn't wrap its result in a proxy. selected being on a reactive object, does. So the comparison item === selected compares an object with its proxy and fails.

@posva Notice how in this case there's no access to a non-ref value exposed by setup.

LinusBorg commented 4 years ago

Sidenote: I wrote a lengthy reply this morning but appearantly i forgot to press "Comment" or something, so here I go again ... :/


computed doesn't wrap its result in a proxy. selected being on a reactive object, does. So the comparison item === selected compares an object with its proxy and fails.

This is correct, and the underlying culprit. And this culprit is not directly linked to wether or not we make setup's return value reactive or not. It's a fundamental tripwire rooted in our use of proxies for reactivity, and we will have to come up with ways (in documentation, tooling etc) to keep people from falling for it.

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 state is made reactive here, whereas for the return value of setup, we do it implicitly, but I would not assume that this will make much of a difference in terms of how many people will* fall for this.

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?

jods4 commented 4 years ago

@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 react but computed that's to blame here.

So let's put that aside.

My previous example from this comment: https://github.com/vuejs/rfcs/issues/121#issuecomment-597966195 and my answer to posva: https://github.com/vuejs/rfcs/issues/121#issuecomment-598121184 are directly related to the magic of calling reactive on setup result.

LinusBorg commented 4 years ago

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 ref when reactive works just as well.

Check out this version which works fine after wrapping the array in reactive.

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 reactive like this. Another way to solve this would be to use toRaw:

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:

  1. all data that we expose to the template will be "reactive",
  2. and therefore a proxy instead of the original value.
  3. So all objects that we get back from the template will be proxies
  4. And so, if we need the raw value, we need to use toRaw

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 proxy !== original mistake - which might be actually more problematic.

jods4 commented 4 years ago

@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 reactive calls yourself where needed. You need to understand what's going on anyway and it's easier to understand when you can see the calls.

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: https://github.com/vuejs/rfcs/issues/129#issuecomment-598131161 The second one is critical to address an issue when restoring alpha 7 behavior from user code, explained in this comment: https://github.com/vuejs/vue-next/commit/e67f655b2687042fcc74dc0993581405abed56de#commitcomment-37690228

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 example

The 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 markNonReactive on it. Not call reactive, that's everything but intuitive.

The solution is then to use a shallowRef around selection:

setup() {     
  const standard = markNonReactive([ Color, Color, Color ])
  return { 
    standard,
    custom: new Color('#0e88e0'), 
    selection: shallowRef(standard[0]),
    isStandard(x) {
      return standard.includes(x)
    },
  }
}
jods4 commented 4 years ago

@LinusBorg

We have to find a general strategy to keep people from stepping into this trap, I think we agree on that.

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. Then you have 2 references, then you have problems. It does so in at least 2 places:

This is very much a opt-in vs opt-out choice and Vue is currently encouraging opt-out. I'm using opt-in in my code by having the following tweaks:

You can stop reading here if this comment is too long, the rest is me commenting on those points 😉


One example in both styles

Let'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 Set that denotes the current selection.

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. Your first try might be:

items.value = markNonReactive(data)

But this is not deeply non-reactive. As soon as one item is taken out of data, put into the selection Set and read back, a proxy of this object will exist. And you'll start having weird identity issues.

Final words

I can't believe users will wrap everything into reactive state. And because of that I'm sure some of them will encounter those funny problem.

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. The deep primitives are very useful, but making them non-default gives an opportunity to teach to devs the differences -- and consequences -- compared to defaults.

LinusBorg commented 4 years ago

@jods4 I appreciate your thoughts on this matter. There's interesting stuff in there, but I disagree pretty fundamentally.

I can't believe users will wrap everything into reactive state.

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 deepReactive: You add an object to your component's data and everything nested in this object will be reactive, no exceptions.

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 fetch finished) would not cause a re-render.

"deep" reactivity as a default is what makes Vue so approachable. As soon as you stick anything into data, it "just works™". You don't actually worry about which object is deeply or shallowly reactive, and what that means for nested objects or arrays - everything just works.

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

  1. better communication of intent by not making reactive that which is not intended to be changed/mutated
  2. Better performance by only creating proxies for those objects that really need reactivity.
  3. "safer defaults" that will keep people from creating proxies in unexpected places, which makes them more prone to run into identity mistakes between proxies and their raw values.

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 readonly(). This

  1. ... communicates intent even more clearly
  2. ... acts as a safeguard against accidental mutations. Mutating a nonreactive object is often a source of bugs were devs wonder why their component is not updating. This actively kills that bug
  3. ...creates proxies. Which will lead us to your second point about performance.

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 default

Here 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? post.categories will return a deeply reactive proxy for the array, which means any item in that array is actually a proxy, which means your computed prop will fail.

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 proposition

My 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:

  1. reactive ("deep")
  2. ref
  3. readonly

...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:

  1. We have a pretty simple rule to follow for the 99% case in which "proxies everywhere" is fine (no performance implications)
  2. The experience with reactivity is similar Vue 2: instead of "add all your state to data" we now have "wrap all your state in one of these 3 functions".
  3. devs get to use deep reactivity as a default like they are used to and expect out of Vue 3.

We can document how to deal with shallowReactive et. al. under the perspective of an escape hatch for performance optimisations when those actually make sense.

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."

jods4 commented 4 years ago

@LinusBorg Thanks for having this discussion. I found your answer interesting and if we were face to face I would have a lot of things to say. To keep this thread manageable I'll focus on the main issue only: how can Vue lead users to a correct usage of proxies.

Your solution (always use either reactive or readonly on state) is actually an application of the principle I coined proxy at creation time and I agree it works even better because there's less risk of putting a raw object into a deep reactive tree inadvertently. I like it.

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 readonly, though, but nothing that can't be fixed easily. Readonly and Non-reactive are not the same thing.

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.

readonly is tracking. Also: readonly prevents mutations. It's not because I don't need to track an object that I may not want to modify it in my code. E.g. when optimizing I could use a signal pattern to trigger updates on top of a mutable, but non-reactive, state.

👉 This is easily fixed by introducing a nonReactive function. A deep wrapper that does nothing.

(A pitfall of readonly is that calling it on a reactive object creates a second proxy. I think it's not a common case and there's not much we can do about that if we want an API to publish readonly views of internal data.)

Assuming everyone wraps all state in either reactive, nonReactive or readonly. Let's revisit this RFC: do we need to call reactive on the returned value of setup? If everything is a proxy, then no. Unwrapping refs from first level is enough.

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 {x, y} is a more consistent thing to do. (1) If only because that's what props does. People will quickly learn that if you want to manipulate props then you must either dereference props.x or use const { x } = toRefs(props). It is more consistent and predictable if all mixins followed suit. (2) Because we said all state should be a proxy, let's not immediately make a huge common exception to the rule.

We can still support this pattern by calling nonReactive (assuming it unwraps refs) instead of reactive on the setup returned value. It performs the same task but does not introduce new tracking calls.

clemyan commented 4 years ago

I'd just like to drop my 2 cents here.

Reactive wrapping

I don't think Vue 2's data behavior should be an argument for wrapping the return value of setup in reactive. Instead, whether we wrap should be guided by developer expectations.

I use Vue 2 professionally, and I do expect data to be deeply reactive. On the other hand, I don't expect, say, methods to be reactive at all, and Vue 2 matches these expectation. The question is how would developers expect setup to behave.

I can only speak for myself here but to me setup() { return { count: 0 } } does not feel reactive. In fact I have written code expecting non-reactivity (debugging code, but still).

For data, we don't use reactive/ref at all but we know the returned object becomes reactive. We understand and expect the magic there. On the other hand, most code examples using setup use reactive/refs explicitly and return an object of refs and functions. Since we have declared reactivity ourselves, it creates the expectation that there is no magic (i.e. additional reactivity) applied to the return value of setup, which does not match the actual behavior.

Ref unwrapping in templates

Based 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 .value in <script> to unref, also needing .value in <template> would be more consistent. That said, I don't feel strongly about unwrap.

If it were up to me I'd remove both auto unref and reactive wrapping, but as a middle ground, the deep hideRefs in the OP sounds like the best solution to me.

jods4 commented 4 years ago

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.