From d4f562a91ac2a70c90bb29ceb29a814d05df40f4 Mon Sep 17 00:00:00 2001 From: pikax Date: Fri, 18 Sep 2020 08:10:38 +0100 Subject: [PATCH 1/5] fix(reactive): fix issue when using reactive `array` in the template --- README.md | 69 ++++++++++++++----------- src/mixin.ts | 45 +++++++++++++++- src/reactivity/reactive.ts | 18 +++++-- test/setup.spec.js | 103 +++++++++++++++++++++++++++++++++++++ 4 files changed, 199 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 719efad6..e9af2902 100644 --- a/README.md +++ b/README.md @@ -6,8 +6,6 @@ Vue 2 plugin for **Composition API** [![GitHub Workflow Status](https://img.shields.io/github/workflow/status/vuejs/composition-api/Build%20&%20Test)](https://github.com/vuejs/composition-api/actions?query=workflow%3A%22Build+%26+Test%22) [![Minzipped size](https://badgen.net/bundlephobia/minzip/@vue/composition-api)](https://bundlephobia.com/result?p=@vue/composition-api) - - English | [中文](./README.zh-CN.md) ・ [**Composition API Docs**](https://composition-api.vuejs.org/) ## Installation @@ -30,7 +28,6 @@ Vue.use(VueCompositionAPI) ``` ```js -// use the APIs import { ref, reactive } from '@vue/composition-api' ``` @@ -41,10 +38,12 @@ import { ref, reactive } from '@vue/composition-api' Include `@vue/composition-api` after Vue and it will install itself automatically. + ```html ``` + `@vue/composition-api` will be exposed to global variable `window.VueCompositionAPI`. @@ -89,7 +88,7 @@ export default { return { result, } - } + }, } ``` @@ -110,16 +109,39 @@ export default { const state = reactive({ list: [ref(0)], }) -// no unwrap, `.value` is required -state.list[0].value === 0 // true +state.list[0].value === 0 state.list.push(ref(1)) -// no unwrap, `.value` is required state.list[1].value === 1 // true ``` +
+ +✅ Should always use ref in a reactive when working with Array + + +```js +const a = reactive({ + list: [ + reactive({ + count: ref(0), + }), + ], +}) +a.list[0].count === 0 + +a.list.push( + reactive({ + count: ref(1), + }) +) +a.list[1].count === 1 // true +``` + +
+
❌ Should NOT use ref in a plain object when working with Array @@ -130,10 +152,9 @@ const a = { count: ref(0), } const b = reactive({ - list: [a], // `a.count` will not unwrap!! + list: [a], }) -// no unwrap for `count`, `.value` is required b.list[0].count.value === 0 // true ``` @@ -141,12 +162,11 @@ b.list[0].count.value === 0 // true const b = reactive({ list: [ { - count: ref(0), // no unwrap!! + count: ref(0), }, ], }) -// no unwrap for `count`, `.value` is required b.list[0].count.value === 0 // true ``` @@ -163,14 +183,13 @@ b.list[0].count.value === 0 // true import { reactive, set } from '@vue/composition-api' const a = reactive({ - foo: 1 + foo: 1, }) // add new reactive key set(a, 'bar', 1) ``` -
### Template Refs @@ -191,8 +210,7 @@ set(a, 'bar', 1) const root = ref(null) onMounted(() => { - // the DOM element will be assigned to the ref after initial render - console.log(root.value) //
+ console.log(root.value) }) return { @@ -216,8 +234,7 @@ export default { const root = ref(null) onMounted(() => { - // the DOM element will be assigned to the ref after initial render - console.log(root.value) //
+ console.log(root.value) }) return { @@ -225,7 +242,6 @@ export default { } }, render() { - // with JSX return () =>
}, } @@ -273,7 +289,6 @@ export default { ref: root, }) - // with JSX return () =>
}, } @@ -297,8 +312,7 @@ export default { setup(initProps, setupContext) { const refs = setupContext.refs onMounted(() => { - // the DOM element will be assigned to the ref after initial render - console.log(refs.root) //
+ console.log(refs.root) }) return () => @@ -306,7 +320,6 @@ export default { ref: 'root', }) - // with JSX return () =>
}, } @@ -333,7 +346,7 @@ declare module '@vue/composition-api' { ⚠️ reactive() mutates the original object -`reactive` uses `Vue.observable` underneath which will ***mutate*** the original object. +`reactive` uses `Vue.observable` underneath which will **_mutate_** the original object. > :bulb: In Vue 3, it will return an new proxy object. @@ -347,11 +360,9 @@ declare module '@vue/composition-api' { ```js -watch(() => { - /* ... */ -}, { +watch(() => {}, { immediate: true, - onTrack() {}, // not available + onTrack() {}, onTrigger() {}, // not available }) ``` @@ -390,6 +401,7 @@ app2.component('Bar', Bar) // equivalent to Vue.use('Bar', Bar) ### `props` +
⚠️ toRefs(props.foo.bar) will incorrectly warn when acessing nested levels of props. @@ -410,8 +422,6 @@ defineComponent({
- - ### Missing APIs The following APIs introduced in Vue 3 are not available in this plugin. @@ -433,7 +443,6 @@ The following APIs introduced in Vue 3 are not available in this plugin. export default { data() { return { - // will result { a: { value: 1 } } in template a: ref(1), } }, diff --git a/src/mixin.ts b/src/mixin.ts index a70c1312..52e38ac9 100644 --- a/src/mixin.ts +++ b/src/mixin.ts @@ -5,7 +5,7 @@ import { SetupFunction, Data, } from './component' -import { isRef, isReactive, toRefs } from './reactivity' +import { isRef, isReactive, toRefs, isRaw } from './reactivity' import { isPlainObject, assert, @@ -24,6 +24,8 @@ import { asVmProperty, } from './utils/instance' import { PropsReactive } from './utils/symbols' +import { isArray } from 'util' +import { getVueConstructor } from './runtimeContext' export function mixin(Vue: VueConstructor) { Vue.mixin({ @@ -121,7 +123,13 @@ export function mixin(Vue: VueConstructor) { bindingValue = bindingValue.bind(vm) } else if (!isObject(bindingValue)) { bindingValue = ref(bindingValue) + } else if (hasReactiveArrayChild(bindingValue)) { + // creates a custom reactive properties without make the object explicitly reactive + // NOTE we should try to avoid this, better implementation needed + customReactive(bindingValue) } + } else if (isArray(bindingValue)) { + bindingValue = ref(bindingValue) } } asVmProperty(vm, name, bindingValue) @@ -140,6 +148,41 @@ export function mixin(Vue: VueConstructor) { } } + function customReactive(target: object) { + if ( + !isPlainObject(target) || + isRef(target) || + isReactive(target) || + isRaw(target) + ) + return + const Vue = getVueConstructor() + const defineReactive = Vue.util.defineReactive + + Object.keys(target).forEach((k) => { + const val = target[k] + defineReactive(target, k, val) + if (val) { + customReactive(val) + } + return + }) + } + + function hasReactiveArrayChild(target: object, visited = new Map()): boolean { + if (visited.has(target)) return visited.get(target) + visited.set(target, false) + if (Array.isArray(target) && isReactive(target)) { + visited.set(target, true) + return true + } + + if (!isPlainObject(target) || isRaw(target)) return false + return Object.keys(target).some((x) => + hasReactiveArrayChild(target[x], visited) + ) + } + function createSetupContext( vm: ComponentInstance & { [x: string]: any } ): SetupContext { diff --git a/src/reactivity/reactive.ts b/src/reactivity/reactive.ts index 8196a583..ae3f7653 100644 --- a/src/reactivity/reactive.ts +++ b/src/reactivity/reactive.ts @@ -1,6 +1,6 @@ import { AnyObject } from '../types/basic' import { getRegisteredVueOrDefault } from '../runtimeContext' -import { isPlainObject, def, warn } from '../utils' +import { isPlainObject, def, warn, isArray } from '../utils' import { isComponentInstance, defineComponentInstance } from '../utils/helper' import { RefKey } from '../utils/symbols' import { isRef, UnwrapRef } from './ref' @@ -121,7 +121,11 @@ export function shallowReactive(obj: any): any { return } - if (!isPlainObject(obj) || isRaw(obj) || !Object.isExtensible(obj)) { + if ( + !(isPlainObject(obj) || isArray(obj)) || + isRaw(obj) || + !Object.isExtensible(obj) + ) { return obj as any } @@ -181,7 +185,11 @@ export function reactive(obj: T): UnwrapRef { return } - if (!isPlainObject(obj) || isRaw(obj) || !Object.isExtensible(obj)) { + if ( + !(isPlainObject(obj) || isArray(obj)) || + isRaw(obj) || + !Object.isExtensible(obj) + ) { return obj as any } @@ -192,7 +200,7 @@ export function reactive(obj: T): UnwrapRef { export function shallowReadonly(obj: T): Readonly export function shallowReadonly(obj: any): any { - if (!isPlainObject(obj) || !Object.isExtensible(obj)) { + if (!(isPlainObject(obj) || isArray(obj)) || !Object.isExtensible(obj)) { return obj } @@ -245,7 +253,7 @@ export function shallowReadonly(obj: any): any { * Make sure obj can't be a reactive */ export function markRaw(obj: T): T { - if (!isPlainObject(obj) || !Object.isExtensible(obj)) { + if (!(isPlainObject(obj) || isArray(obj)) || !Object.isExtensible(obj)) { return obj } diff --git a/test/setup.spec.js b/test/setup.spec.js index 492feab0..a9657dde 100644 --- a/test/setup.spec.js +++ b/test/setup.spec.js @@ -886,4 +886,107 @@ describe('setup', () => { await nextTick() expect(vm.$el.textContent).toBe('2') }) + + // #524 + it('should work with reactive arrays.', async () => { + const opts = { + template: `
{{items.length}}
`, + setup() { + const items = reactive([]) + + setTimeout(() => { + items.push(2) + }, 1) + + return { + items, + } + }, + } + const Constructor = Vue.extend(opts).extend({}) + + const vm = new Vue(Constructor).$mount() + expect(vm.$el.textContent).toBe('0') + await sleep(10) + await nextTick() + expect(vm.$el.textContent).toBe('1') + }) + + it('should work with reactive array nested', async () => { + const opts = { + template: `
{{a.items.length}}
`, + setup() { + const items = reactive([]) + + setTimeout(() => { + items.push(2) + }, 1) + + return { + a: { + items, + }, + } + }, + } + const Constructor = Vue.extend(opts).extend({}) + + const vm = new Vue(Constructor).$mount() + expect(vm.$el.textContent).toBe('0') + await sleep(10) + await nextTick() + expect(vm.$el.textContent).toBe('1') + }) + + it('should not unwrap reactive array nested', async () => { + const opts = { + template: `
{{a.items}}
`, + setup() { + const items = reactive([]) + + setTimeout(() => { + items.push(ref(1)) + }, 1) + + return { + a: { + items, + }, + } + }, + } + const Constructor = Vue.extend(opts).extend({}) + + const vm = new Vue(Constructor).$mount() + expect(vm.$el.textContent).toBe('[]') + await sleep(10) + await nextTick() + expect(JSON.parse(vm.$el.textContent)).toStrictEqual([{ value: 1 }]) + }) + + // TODO make this pass + // it('should work with computed', async ()=>{ + // const opts = { + // template: `
{{len}}
`, + // setup() { + // const array = reactive([]); + // const len = computed(()=> array.length); + + // setTimeout(() => { + // array.push(2) + // }, 1) + + // return { + // len + // } + // }, + // } + // const Constructor = Vue.extend(opts).extend({}) + + // const vm = new Vue(Constructor).$mount() + // expect(vm.$el.textContent).toBe('0') + // await sleep(10) + // await nextTick() + // expect(vm.$el.textContent).toBe('1') + // }) }) From 87b4507f71cc55d31342a57d1121f90c1759f82c Mon Sep 17 00:00:00 2001 From: pikax Date: Fri, 18 Sep 2020 10:13:37 +0100 Subject: [PATCH 2/5] chore: rollback readme --- README.md | 96 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 57 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index e9af2902..2ebd041e 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,8 @@ Vue 2 plugin for **Composition API** [![GitHub Workflow Status](https://img.shields.io/github/workflow/status/vuejs/composition-api/Build%20&%20Test)](https://github.com/vuejs/composition-api/actions?query=workflow%3A%22Build+%26+Test%22) [![Minzipped size](https://badgen.net/bundlephobia/minzip/@vue/composition-api)](https://bundlephobia.com/result?p=@vue/composition-api) + + English | [中文](./README.zh-CN.md) ・ [**Composition API Docs**](https://composition-api.vuejs.org/) ## Installation @@ -28,6 +30,7 @@ Vue.use(VueCompositionAPI) ``` ```js +// use the APIs import { ref, reactive } from '@vue/composition-api' ``` @@ -38,12 +41,10 @@ import { ref, reactive } from '@vue/composition-api' Include `@vue/composition-api` after Vue and it will install itself automatically. - ```html ``` - `@vue/composition-api` will be exposed to global variable `window.VueCompositionAPI`. @@ -88,7 +89,7 @@ export default { return { result, } - }, + } } ``` @@ -109,39 +110,16 @@ export default { const state = reactive({ list: [ref(0)], }) -state.list[0].value === 0 +// no unwrap, `.value` is required +state.list[0].value === 0 // true state.list.push(ref(1)) +// no unwrap, `.value` is required state.list[1].value === 1 // true ``` -
- -✅ Should always use ref in a reactive when working with Array - - -```js -const a = reactive({ - list: [ - reactive({ - count: ref(0), - }), - ], -}) -a.list[0].count === 0 - -a.list.push( - reactive({ - count: ref(1), - }) -) -a.list[1].count === 1 // true -``` - -
-
❌ Should NOT use ref in a plain object when working with Array @@ -152,9 +130,10 @@ const a = { count: ref(0), } const b = reactive({ - list: [a], + list: [a], // `a.count` will not unwrap!! }) +// no unwrap for `count`, `.value` is required b.list[0].count.value === 0 // true ``` @@ -162,16 +141,44 @@ b.list[0].count.value === 0 // true const b = reactive({ list: [ { - count: ref(0), + count: ref(0), // no unwrap!! }, ], }) +// no unwrap for `count`, `.value` is required b.list[0].count.value === 0 // true ```
+
+ +✅ Should always use ref in a reactive when working with Array + + +```js +const a = reactive({ + list: [ + reactive({ + count: ref(0), + }), + ] +}) +// unwrapped +a.list[0].count === 0 // true + +a.list.push( + reactive({ + count: ref(1), + }) +) +// unwrapped +a.list[1].count === 1 // true +``` + +
+
⚠️ `set` workaround for adding new reactive properties @@ -183,13 +190,14 @@ b.list[0].count.value === 0 // true import { reactive, set } from '@vue/composition-api' const a = reactive({ - foo: 1, + foo: 1 }) // add new reactive key set(a, 'bar', 1) ``` +
### Template Refs @@ -210,7 +218,8 @@ set(a, 'bar', 1) const root = ref(null) onMounted(() => { - console.log(root.value) + // the DOM element will be assigned to the ref after initial render + console.log(root.value) //
}) return { @@ -234,7 +243,8 @@ export default { const root = ref(null) onMounted(() => { - console.log(root.value) + // the DOM element will be assigned to the ref after initial render + console.log(root.value) //
}) return { @@ -242,6 +252,7 @@ export default { } }, render() { + // with JSX return () =>
}, } @@ -289,6 +300,7 @@ export default { ref: root, }) + // with JSX return () =>
}, } @@ -312,7 +324,8 @@ export default { setup(initProps, setupContext) { const refs = setupContext.refs onMounted(() => { - console.log(refs.root) + // the DOM element will be assigned to the ref after initial render + console.log(refs.root) //
}) return () => @@ -320,6 +333,7 @@ export default { ref: 'root', }) + // with JSX return () =>
}, } @@ -346,7 +360,7 @@ declare module '@vue/composition-api' { ⚠️ reactive() mutates the original object -`reactive` uses `Vue.observable` underneath which will **_mutate_** the original object. +`reactive` uses `Vue.observable` underneath which will ***mutate*** the original object. > :bulb: In Vue 3, it will return an new proxy object. @@ -360,9 +374,11 @@ declare module '@vue/composition-api' { ```js -watch(() => {}, { +watch(() => { + /* ... */ +}, { immediate: true, - onTrack() {}, + onTrack() {}, // not available onTrigger() {}, // not available }) ``` @@ -401,7 +417,6 @@ app2.component('Bar', Bar) // equivalent to Vue.use('Bar', Bar) ### `props` -
⚠️ toRefs(props.foo.bar) will incorrectly warn when acessing nested levels of props. @@ -422,6 +437,8 @@ defineComponent({
+ + ### Missing APIs The following APIs introduced in Vue 3 are not available in this plugin. @@ -443,6 +460,7 @@ The following APIs introduced in Vue 3 are not available in this plugin. export default { data() { return { + // will result { a: { value: 1 } } in template a: ref(1), } }, From 6f5644218c830634d93330d52848ce64620db9df Mon Sep 17 00:00:00 2001 From: Anthony Fu Date: Thu, 1 Oct 2020 13:50:40 +0800 Subject: [PATCH 3/5] Update src/mixin.ts --- src/mixin.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mixin.ts b/src/mixin.ts index 52e38ac9..ac923ad0 100644 --- a/src/mixin.ts +++ b/src/mixin.ts @@ -170,7 +170,9 @@ export function mixin(Vue: VueConstructor) { } function hasReactiveArrayChild(target: object, visited = new Map()): boolean { - if (visited.has(target)) return visited.get(target) + if (visited.has(target)) { + return visited.get(target) + } visited.set(target, false) if (Array.isArray(target) && isReactive(target)) { visited.set(target, true) From 605e105f1e4b28df7c76a0ad014c307643027efa Mon Sep 17 00:00:00 2001 From: Anthony Fu Date: Thu, 1 Oct 2020 13:51:32 +0800 Subject: [PATCH 4/5] Update src/mixin.ts --- src/mixin.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mixin.ts b/src/mixin.ts index ac923ad0..8f69c4e8 100644 --- a/src/mixin.ts +++ b/src/mixin.ts @@ -179,7 +179,9 @@ export function mixin(Vue: VueConstructor) { return true } - if (!isPlainObject(target) || isRaw(target)) return false + if (!isPlainObject(target) || isRaw(target)) { + return false + } return Object.keys(target).some((x) => hasReactiveArrayChild(target[x], visited) ) From 1ae493969a7873dfccc1063d49c189004183a4b1 Mon Sep 17 00:00:00 2001 From: pikax Date: Sun, 4 Oct 2020 09:13:59 +0100 Subject: [PATCH 5/5] chore: fix imports --- src/mixin.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/mixin.ts b/src/mixin.ts index 6d821666..17fe2e80 100644 --- a/src/mixin.ts +++ b/src/mixin.ts @@ -14,6 +14,7 @@ import { isFunction, isObject, def, + isArray, } from './utils' import { ref } from './apis' import vmStateManager from './utils/vmStateManager' @@ -23,8 +24,6 @@ import { resolveScopedSlots, asVmProperty, } from './utils/instance' -import { PropsReactive } from './utils/symbols' -import { isArray } from 'util' import { getVueConstructor } from './runtimeContext' import { createObserver } from './reactivity/reactive'