warn if $set is used on a property that already exist

[JJPandari]

Version

2.5.16

Reproduction link

https://codepen.io/JJPandari/pen/gzLVBq?editors=1010

Steps to reproduce

See the codepen snippet. Follow the comment there to change the vm's data and see what happens.

What is expected?

Even if the prop already exists, using set still makes it reactive, thus trigger view update.

What is actually happening?

Using set later doesn't update the view.

---

Related source code:

vue/src/core/observer/index.js

Line 192 in 3eb37ac

* Set a property on an object. Adds the new property and

I think most users would expect set to make the prop reactive whenever it's used. I initially opened an issue for the api doc because it wasn't clear (for me) about this. But the comment in the source code is.

[posva]

You're setting 2 different setCameLate, your last line should be

this.$set(this, 'setCameLate', 'yes');

---

Please, next time consider using the forum, the Discord server or StackOverflow for questions first. But feel free to come back and open an issue if it turns out to be a bug 🙂

[JJPandari]

Sorry I messed the example up. Try read it again. Or you can read the source code of set I linked to above briefly, it should be easy to see what I'm talking about in the title of this issue. I'm asking about a design decision here, "why not (do it the other way)?"

[posva]

Im not sure I get your question, but vue cannot detect the assignment, so you need set.

[chrisvfritz]

@posva I think what @JJPandari would like to do is use Vue.set to make a property reactive after it's already been added, like in this example. I don't see a use case for this, but if the property already exists, I think it would be good to provide a development warning to users, so they know they've done something wrong. For example, for:

Vue.set(this.person, 'job', 'Educator')

If this.person.job had previously been created without Vue.set, so that it's non-reactive, then I think a warning like this would be useful:

[Vue warn]: You tried to use Vue.set on the existing, non-reactive property "job". Properties cannot be made reactive after they've already been added to an object. Instead, use Vue.set where you want to initially create that property.

What do you think?

[posva]

seems fine. I also think it's hardly useful but a warning is ok

[bigtunacan]

@chrisvfritz Would you see this being logged to the browser console, or something that would be in the build process? Just thinking about taking a stab at this.

[posva]

it's a runtime dev only warning if you want to add it 🙂

[fnlctrl]

I think there's a valid use case for setting $set on existing properties. In my application code I have a hashMap object that is initially empty, and I call this.$set(hashMap, object.id, object) after getting the object from API. It will be called more than once if I get updated object sent from API again (apiCall().then(object => this.$set(hashMap, object.id, object))). I don't really want to check if the property exist s before using $set since that would be too verbose.

[bigtunacan]

@fnlctrl It's a bit unclear the way this issue was initially worded, but Chris' codepen is a good example of the real issue here. https://codepen.io/chrisvfritz/pen/rvzgBR?editors=1010

JJPandari's real issue was that they created a non-reactive property then later called $set on the non-reactive property expecting that $set would switch the property from non-reactive to reactive.

The pull request I added on this #8138 is also addressing only that usage of a call to set. In a non-production environment, it will check if you are trying to set a non-reactive property in which case you would get a warning that said property will not be reactive.

[fnlctrl]

I see... though there's another question: Why not just make it work too when there's already a non-reactive property? 😄 I don't see why there should be a limit.

[chrisvfritz]

@fnlctrl My thinking was that if someone first creates an unreactive property, then tries to make it reactive later, any reference to that property in between those two events is very likely to create a difficult-to-diagnose bug.

By showing a warning that they should make the property reactive from the start, we encourage a best practice that eliminates the window for bugs to occur.

[alecat88]

this issue seems close to me, why does it still say "Open" near the title?

[JJPandari]

It's probably fixed but I'm not sure, haven't used vue recently. @posva Would you link the PR to this issue and close this if fixed?

[DaZuiZui]

Does this problem still exist?