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

api doc: explicitly tell set only works when the prop doesn't exist yet #1601

Closed
JJPandari opened this issue Apr 27, 2018 · 7 comments
Closed

Comments

@JJPandari
Copy link

JJPandari commented Apr 27, 2018

Note: Here I'm all talking about a prop that doesn't exist in data in the first place.

The api says:

If the object is reactive, ensure the property is created as a reactive property and trigger view updates.

At least for me, it's not clear "does set work if the prop isn't in data, and I've already added it diredtly without set?". So I tried the source code and the comment made it clear:

Adds the new property and triggers change notification if the property doesn't already exist.

Then I altered my code to use set at prop addition and the not-properly-re-rendering problem was fixed.

So I think it may be better to explicitly tell in the api that set only works when the prop doesn't exist yet? Like the comment does?

@chrisvfritz
Copy link
Contributor

Set should also work with a property that already exists, like here - it's just not necessary. Could you provide a pen/fiddle demonstrating the situation you found where it doesn't work?

@JJPandari
Copy link
Author

@chrisvfritz Here we go. I'm not entirely clear in the first post, edited it, try read again.

@chrisvfritz
Copy link
Contributor

Ah, I see the issue here. It's:

I've already added it directly without set

The reason set exists is because you can't add new, reactive properties without it. And in the docs, we specify that Vue.set is only for property creation/addition:

Set a property on an object. If the object is reactive, ensure the property is created as a reactive property and trigger view updates. This is primarily used to get around the limitation that Vue cannot detect property additions.

To me, that already specifies that set is only for new (non-existing) properties, but I'm also open to alternate phrasing or adding an example if you think that would be helpful. What are your thoughts? 🙂


On a related note, it's bad practice to add new root properties to $data outside of the data function and Vue actually warns you about this in the console:

[Vue warn]: Avoid adding reactive properties to a Vue instance or its root $data at runtime - declare it upfront in the data option.

@JJPandari
Copy link
Author

JJPandari commented May 2, 2018

Of the phrasing, I'm not sure, I'm Chinese and your name suggests you're a native English speaker so... I feel: "created", can I recreated it with $set? Because I don't need view update when the prop is added, but do when I used $set. "cannot detect addition", sure, can I make up for it using $set after addition? Feels somehow vague, but I'm not sure if it's necessary to stress using $set later won't help. Maybe we should wait for someone else's opinion.


Not declaring in data is obviously not good practice and can be found wrong (not only reduce readability) with the help of the warning, but my actual use case is kind of like adding to an object in an array in data, where it's a bit harder to avoid, because in data declaration I can only write foo: [].

@chrisvfritz
Copy link
Contributor

Hm, what would you think about this rewording?

Adds a property on a reactive object, ensuring the new property is also reactive, so triggers view updates. This must be used to add new properties to reactive objects, as Vue cannot detect normal property additions (e.g. myObject.newProperty = 'hi').

@JJPandari
Copy link
Author

I think this is more clear if anyone has the same doubt as me:

...This must be used at new property addition, using it after normal property addition (e.g. myObject.newProperty = 'hi') won't make the property reactive.

Somehow I feel this behavior itself is not intuitive. I've opened an issue for vue, let's see what people say about that then.

@chrisvfritz
Copy link
Contributor

Excellent! I just made that update to the docs. I also left a comment on your issue. I don't think Vue.set should work to make existing properties reactive, because not doing it initially leaves open a window for bugs. I do agree we can improve the experience, however, and suggested that Vue emit a warning instead so that users know what they've done wrong. 🙂

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

No branches or pull requests

2 participants