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.

Sign up for GitHub

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

Jump to bottom

injected objects are reactive (despite documentation saying the opposite) #5913

Closed
akarelas opened this issue Jun 18, 2017 · 10 comments
Closed

injected objects are reactive (despite documentation saying the opposite) #5913

akarelas opened this issue Jun 18, 2017 · 10 comments
Labels
regression

Comments

@akarelas
Copy link

Version

2.3.3

Reproduction link

https://jsfiddle.net/50wL7mdz/41000/

Steps to reproduce

Provide (from parent component) an object with a pre-existing key. Inject that object into child component. Either console.log the object to see that it is reactive, or modify the value of the pre-existing key to see that the value also changes on screen automatically.

What is expected?

According to vue's documentation, injected objects are not reactive: "Note: the provide and inject bindings are NOT reactive.". The value should not change on the screen after 3 seconds, and console.log should not display the word Getter when logging the injected object.

What is actually happening?

The injected object is reactive, and so the message changes on screen after 3 seconds, and console.log'ging the object shows it is reactive (displays the word Getter)

I use inject to inject commonly used libraries in objects, like moment, which I can then use in the templates to display nicely formatted dates. If the entire moment library object becomes reactive, then this will probably cause an unnecessary performance hit to the browser when using the moment library.
@LinusBorg
Copy link
Member

LinusBorg commented Jun 18, 2017 •
edited

Hm, indeed. Thanks for reporting this!

It seems #5204 introduced this, but I don't think this was intended.

@yyx990803 I think this is a regression from the original, intended behaviour of inject - would you agree?

@Kingwl
Copy link
Member

Kingwl commented Jun 19, 2017 •
edited

thanks for reporting this!
@LinusBorg actual #5229 introduced this
it seems a regression...my fault

@Darival
Copy link

Darival commented Jun 19, 2017 •
edited

I'm glad I found this, I was pulling my hair off.

Is this the same that is happening to me?

https://jsfiddle.net/50wL7mdz/41229/

look how updating childObj.msg in component also updates someObj.msg in parent T.T

@Kingwl
Copy link
Member

Kingwl commented Jun 20, 2017 •
edited

@Darival in #5229

We can just use that to do the warning when a child component attempts to mutate an injection binding. This is what we are doing for props too

IMO you shouldn't mutate an injection the same as react context....

@rayrutjes
Copy link
Contributor

Personally I considered this change as a feature more than a bug.

@LinusBorg
Copy link
Member

@rayrutjes the injected properties themselves were left unreactive pretty much for the reason OP provided: So you can inject arbitrary objects, inlcuding complicated ones, in the background and have them left untouched.

If you need reactivity, just pass down an already reactive object - best of both worlds. ;)

@Kingwl
Copy link
Member

Kingwl commented Jun 20, 2017 •
edited

@LinusBorg
but the injected array will not changed even it is really reactive...

@rayrutjes
Copy link
Contributor

@LinusBorg I think that makes sense. Wondering how this will affect our existing code base ;)

@LinusBorg
Copy link
Member

@Kingwl Could you make a PR to revert this behaviour?

@Kingwl
Copy link
Member

Kingwl commented Jul 7, 2017

@LinusBorg i'm going to try it tomorrow...

Kingwl added a commit to Kingwl/vue that referenced this issue Jul 9, 2017
@Kingwl
fix(core): injections should not always reactive #(5913)
e3e25ff 
fix vuejs#5913
@Kingwl Kingwl mentioned this issue Jul 9, 2017
Closed
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
regression
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(core): injections should not always reactive #(5913) Kingwl/vue
5 participants
@akarelas @LinusBorg @rayrutjes @Kingwl @Darival