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

fix: unwrap refs returned by data #387

Merged
merged 2 commits into from Jun 19, 2020
Merged

Conversation

pikax
Copy link
Member

@pikax pikax commented Jun 16, 2020

fix #385

I have a few concerns here, because the unwrapping will create a new object everytime:

const myObject = { a: 1}

const vm = new Vue({     data() {
        return {
          myObject,
        }
      },
    }).$mount()

vm.myObject === myObject; // this will always be false

Not sure how much of a deal breaker this might be, one way to do this is only unwrap if there's a ref anywhere in the object.

The reason we create a new object is because we don't want to mutate the original object the user is using in the setup, because that will lead to some unexpected bugs

@pikax pikax requested review from LinusBorg and antfu June 16, 2020 07:41
@antfu
Copy link
Member

antfu commented Jun 16, 2020

Thanks for make PR so fast as always!

Just tested out with vue-next, I am surprised to see that vue-next does handle ref in data. (which makes setup and data almost the same to me). Since vue-next does that, I think it's good to have it in this plugin as well.

About the deep cloning, I think it's ok in most of the common cases. One thing I am worried about is the performance impact when passing a huge object. That would require users to use Object.freeze for them.

@pikax
Copy link
Member Author

pikax commented Jun 16, 2020

My concerns is using the template to change the object values :/

@antfu
Copy link
Member

antfu commented Jun 16, 2020

template to change the object values

Sorry, I can't imagine a case right out of my mind. Can you show me an example about that?

@pikax
Copy link
Member Author

pikax commented Jun 16, 2020

The case I was thinking was this: https://github.com/vuejs/composition-api/pull/387/files#diff-de44c4f0babbf8fdd103f1756b4b7dd9R748

But seems to work just fine

@antfu antfu merged commit 1f07075 into vuejs:master Jun 19, 2020
antfu pushed a commit to antfu/composition-api that referenced this pull request Jun 21, 2020
@antfu antfu mentioned this pull request Jun 21, 2020
antfu added a commit that referenced this pull request Jun 21, 2020
* Revert "fix: unwrap refs returned by `data` (#387)"

This reverts commit 1f07075.

* docs: add limitation note for reactive apis in data()

Co-authored-by: Carlos Rodrigues <david-181@hotmail.com>
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

Successfully merging this pull request may close these issues.

data() does not auto unwrap ref in template
2 participants