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

feat: proxyRefs method and ShallowUnwrapRefs type #456

Merged
merged 5 commits into from
Aug 6, 2020

Conversation

pikax
Copy link
Member

@pikax pikax commented Aug 4, 2020

BREAKING CHANGE: template auto ref unwrapping are now applied shallowly,
i.e. only at the root level. See vuejs/core#1682 for
more details.
@pikax pikax requested a review from antfu August 4, 2020 18:07
expect(warn).not.toHaveBeenCalled()
expect(vm.$el.textContent).toBe('42')
})
// it('not warn doing toRef on props', async () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is caused because the props are marked as raw

Open for suggestions

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess we can mark the props to shallow reactive and change the test to only tests the top-level toRefs, just like you mentioned in the limitation part?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Props are already reactive, only the nested ref are not

it('toRefs(props) should not warn', async () => {
let a
const child = {
template: `<div/>`,
props: {
r: Number,
},
setup(props) {
a = toRefs(props).r
},
}
const vm = new Vue({
template: `<child :r="r"/>`,
components: {
child,
},
data() {
return {
r: 1,
}
},
}).$mount()
expect(a.value).toBe(1)
vm.r = 3
await Vue.nextTick()
expect(a.value).toBe(3)
expect(warn).not.toHaveBeenCalled()
})

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine by leaving a note in the limitation for this.

README.md Outdated Show resolved Hide resolved
src/mixin.ts Outdated Show resolved Hide resolved
pikax and others added 2 commits August 6, 2020 09:06
Co-authored-by: Anthony Fu <[email protected]>
Co-authored-by: Anthony Fu <[email protected]>
src/reactivity/ref.ts Outdated Show resolved Hide resolved
@pikax pikax merged commit 149821a into vuejs:master Aug 6, 2020
@pikax pikax deleted the feat/setup-unwrap branch August 6, 2020 08:34
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.

2 participants