Skip to content

Commit

Permalink
refactor(reactivity): adjust APIs
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Reactivity APIs adjustments:

- `readonly` is now non-tracking if called on plain objects.
  `lock` and `unlock` have been removed. A `readonly` proxy can no
  longer be directly mutated. However, it can still wrap an already
  reactive object and track changes to the source reactive object.

- `isReactive` now only returns true for proxies created by `reactive`,
   or a `readonly` proxy that wraps a `reactive` proxy.

- A new utility `isProxy` is introduced, which returns true for both
  reactive or readonly proxies.

- `markNonReactive` has been renamed to `markRaw`.
  • Loading branch information
yyx990803 committed Apr 15, 2020
1 parent 11654a6 commit 09b4202
Show file tree
Hide file tree
Showing 11 changed files with 139 additions and 125 deletions.
6 changes: 3 additions & 3 deletions packages/reactivity/__tests__/effect.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
TrackOpTypes,
TriggerOpTypes,
DebuggerEvent,
markNonReactive,
markRaw,
ref
} from '../src/index'
import { ITERATE_KEY } from '../src/effect'
Expand Down Expand Up @@ -732,9 +732,9 @@ describe('reactivity/effect', () => {
expect(dummy).toBe(3)
})

it('markNonReactive', () => {
it('markRaw', () => {
const obj = reactive({
foo: markNonReactive({
foo: markRaw({
prop: 0
})
})
Expand Down
6 changes: 3 additions & 3 deletions packages/reactivity/__tests__/reactive.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
reactive,
isReactive,
toRaw,
markNonReactive,
markRaw,
shallowReactive
} from '../src/reactive'
import { mockWarn } from '@vue/shared'
Expand Down Expand Up @@ -146,10 +146,10 @@ describe('reactivity/reactive', () => {
expect(reactive(d)).toBe(d)
})

test('markNonReactive', () => {
test('markRaw', () => {
const obj = reactive({
foo: { a: 1 },
bar: markNonReactive({ b: 2 })
bar: markRaw({ b: 2 })
})
expect(isReactive(obj.foo)).toBe(true)
expect(isReactive(obj.bar)).toBe(false)
Expand Down
176 changes: 92 additions & 84 deletions packages/reactivity/__tests__/readonly.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import {
toRaw,
isReactive,
isReadonly,
markNonReactive,
markRaw,
effect,
ref,
shallowReadonly
shallowReadonly,
isProxy
} from '../src'
import { mockWarn } from '@vue/shared'

Expand All @@ -22,22 +23,23 @@ describe('reactivity/readonly', () => {
describe('Object', () => {
it('should make nested values readonly', () => {
const original = { foo: 1, bar: { baz: 2 } }
const observed = readonly(original)
expect(observed).not.toBe(original)
expect(isReactive(observed)).toBe(true)
expect(isReadonly(observed)).toBe(true)
const wrapped = readonly(original)
expect(wrapped).not.toBe(original)
expect(isProxy(wrapped)).toBe(true)
expect(isReactive(wrapped)).toBe(false)
expect(isReadonly(wrapped)).toBe(true)
expect(isReactive(original)).toBe(false)
expect(isReadonly(original)).toBe(false)
expect(isReactive(observed.bar)).toBe(true)
expect(isReadonly(observed.bar)).toBe(true)
expect(isReactive(wrapped.bar)).toBe(false)
expect(isReadonly(wrapped.bar)).toBe(true)
expect(isReactive(original.bar)).toBe(false)
expect(isReadonly(original.bar)).toBe(false)
// get
expect(observed.foo).toBe(1)
expect(wrapped.foo).toBe(1)
// has
expect('foo' in observed).toBe(true)
expect('foo' in wrapped).toBe(true)
// ownKeys
expect(Object.keys(observed)).toEqual(['foo', 'bar'])
expect(Object.keys(wrapped)).toEqual(['foo', 'bar'])
})

it('should not allow mutation', () => {
Expand All @@ -49,54 +51,54 @@ describe('reactivity/readonly', () => {
},
[qux]: 3
}
const observed: Writable<typeof original> = readonly(original)
const wrapped: Writable<typeof original> = readonly(original)

observed.foo = 2
expect(observed.foo).toBe(1)
wrapped.foo = 2
expect(wrapped.foo).toBe(1)
expect(
`Set operation on key "foo" failed: target is readonly.`
).toHaveBeenWarnedLast()

observed.bar.baz = 3
expect(observed.bar.baz).toBe(2)
wrapped.bar.baz = 3
expect(wrapped.bar.baz).toBe(2)
expect(
`Set operation on key "baz" failed: target is readonly.`
).toHaveBeenWarnedLast()

observed[qux] = 4
expect(observed[qux]).toBe(3)
wrapped[qux] = 4
expect(wrapped[qux]).toBe(3)
expect(
`Set operation on key "Symbol(qux)" failed: target is readonly.`
).toHaveBeenWarnedLast()

delete observed.foo
expect(observed.foo).toBe(1)
delete wrapped.foo
expect(wrapped.foo).toBe(1)
expect(
`Delete operation on key "foo" failed: target is readonly.`
).toHaveBeenWarnedLast()

delete observed.bar.baz
expect(observed.bar.baz).toBe(2)
delete wrapped.bar.baz
expect(wrapped.bar.baz).toBe(2)
expect(
`Delete operation on key "baz" failed: target is readonly.`
).toHaveBeenWarnedLast()

delete observed[qux]
expect(observed[qux]).toBe(3)
delete wrapped[qux]
expect(wrapped[qux]).toBe(3)
expect(
`Delete operation on key "Symbol(qux)" failed: target is readonly.`
).toHaveBeenWarnedLast()
})

it('should not trigger effects', () => {
const observed: any = readonly({ a: 1 })
const wrapped: any = readonly({ a: 1 })
let dummy
effect(() => {
dummy = observed.a
dummy = wrapped.a
})
expect(dummy).toBe(1)
observed.a = 2
expect(observed.a).toBe(1)
wrapped.a = 2
expect(wrapped.a).toBe(1)
expect(dummy).toBe(1)
expect(`target is readonly`).toHaveBeenWarned()
})
Expand All @@ -105,65 +107,66 @@ describe('reactivity/readonly', () => {
describe('Array', () => {
it('should make nested values readonly', () => {
const original = [{ foo: 1 }]
const observed = readonly(original)
expect(observed).not.toBe(original)
expect(isReactive(observed)).toBe(true)
expect(isReadonly(observed)).toBe(true)
const wrapped = readonly(original)
expect(wrapped).not.toBe(original)
expect(isProxy(wrapped)).toBe(true)
expect(isReactive(wrapped)).toBe(false)
expect(isReadonly(wrapped)).toBe(true)
expect(isReactive(original)).toBe(false)
expect(isReadonly(original)).toBe(false)
expect(isReactive(observed[0])).toBe(true)
expect(isReadonly(observed[0])).toBe(true)
expect(isReactive(wrapped[0])).toBe(false)
expect(isReadonly(wrapped[0])).toBe(true)
expect(isReactive(original[0])).toBe(false)
expect(isReadonly(original[0])).toBe(false)
// get
expect(observed[0].foo).toBe(1)
expect(wrapped[0].foo).toBe(1)
// has
expect(0 in observed).toBe(true)
expect(0 in wrapped).toBe(true)
// ownKeys
expect(Object.keys(observed)).toEqual(['0'])
expect(Object.keys(wrapped)).toEqual(['0'])
})

it('should not allow mutation', () => {
const observed: any = readonly([{ foo: 1 }])
observed[0] = 1
expect(observed[0]).not.toBe(1)
const wrapped: any = readonly([{ foo: 1 }])
wrapped[0] = 1
expect(wrapped[0]).not.toBe(1)
expect(
`Set operation on key "0" failed: target is readonly.`
).toHaveBeenWarned()
observed[0].foo = 2
expect(observed[0].foo).toBe(1)
wrapped[0].foo = 2
expect(wrapped[0].foo).toBe(1)
expect(
`Set operation on key "foo" failed: target is readonly.`
).toHaveBeenWarned()

// should block length mutation
observed.length = 0
expect(observed.length).toBe(1)
expect(observed[0].foo).toBe(1)
wrapped.length = 0
expect(wrapped.length).toBe(1)
expect(wrapped[0].foo).toBe(1)
expect(
`Set operation on key "length" failed: target is readonly.`
).toHaveBeenWarned()

// mutation methods invoke set/length internally and thus are blocked as well
observed.push(2)
expect(observed.length).toBe(1)
wrapped.push(2)
expect(wrapped.length).toBe(1)
// push triggers two warnings on [1] and .length
expect(`target is readonly.`).toHaveBeenWarnedTimes(5)
})

it('should not trigger effects', () => {
const observed: any = readonly([{ a: 1 }])
const wrapped: any = readonly([{ a: 1 }])
let dummy
effect(() => {
dummy = observed[0].a
dummy = wrapped[0].a
})
expect(dummy).toBe(1)
observed[0].a = 2
expect(observed[0].a).toBe(1)
wrapped[0].a = 2
expect(wrapped[0].a).toBe(1)
expect(dummy).toBe(1)
expect(`target is readonly`).toHaveBeenWarnedTimes(1)
observed[0] = { a: 2 }
expect(observed[0].a).toBe(1)
wrapped[0] = { a: 2 }
expect(wrapped[0].a).toBe(1)
expect(dummy).toBe(1)
expect(`target is readonly`).toHaveBeenWarnedTimes(2)
})
Expand All @@ -176,14 +179,15 @@ describe('reactivity/readonly', () => {
const key1 = {}
const key2 = {}
const original = new Collection([[key1, {}], [key2, {}]])
const observed = readonly(original)
expect(observed).not.toBe(original)
expect(isReactive(observed)).toBe(true)
expect(isReadonly(observed)).toBe(true)
const wrapped = readonly(original)
expect(wrapped).not.toBe(original)
expect(isProxy(wrapped)).toBe(true)
expect(isReactive(wrapped)).toBe(false)
expect(isReadonly(wrapped)).toBe(true)
expect(isReactive(original)).toBe(false)
expect(isReadonly(original)).toBe(false)
expect(isReactive(observed.get(key1))).toBe(true)
expect(isReadonly(observed.get(key1))).toBe(true)
expect(isReactive(wrapped.get(key1))).toBe(false)
expect(isReadonly(wrapped.get(key1))).toBe(true)
expect(isReactive(original.get(key1))).toBe(false)
expect(isReadonly(original.get(key1))).toBe(false)
})
Expand All @@ -209,15 +213,15 @@ describe('reactivity/readonly', () => {
const key1 = {}
const key2 = {}
const original = new Collection([[key1, {}], [key2, {}]])
const observed: any = readonly(original)
for (const [key, value] of observed) {
const wrapped: any = readonly(original)
for (const [key, value] of wrapped) {
expect(isReadonly(key)).toBe(true)
expect(isReadonly(value)).toBe(true)
}
observed.forEach((value: any) => {
wrapped.forEach((value: any) => {
expect(isReadonly(value)).toBe(true)
})
for (const value of observed.values()) {
for (const value of wrapped.values()) {
expect(isReadonly(value)).toBe(true)
}
})
Expand All @@ -232,13 +236,14 @@ describe('reactivity/readonly', () => {
const key1 = {}
const key2 = {}
const original = new Collection([key1, key2])
const observed = readonly(original)
expect(observed).not.toBe(original)
expect(isReactive(observed)).toBe(true)
expect(isReadonly(observed)).toBe(true)
const wrapped = readonly(original)
expect(wrapped).not.toBe(original)
expect(isProxy(wrapped)).toBe(true)
expect(isReactive(wrapped)).toBe(false)
expect(isReadonly(wrapped)).toBe(true)
expect(isReactive(original)).toBe(false)
expect(isReadonly(original)).toBe(false)
expect(observed.has(reactive(key1))).toBe(true)
expect(wrapped.has(reactive(key1))).toBe(true)
expect(original.has(reactive(key1))).toBe(false)
})

Expand All @@ -261,17 +266,17 @@ describe('reactivity/readonly', () => {
if (Collection === Set) {
test('should retrieve readonly values on iteration', () => {
const original = new Collection([{}, {}])
const observed: any = readonly(original)
for (const value of observed) {
const wrapped: any = readonly(original)
for (const value of wrapped) {
expect(isReadonly(value)).toBe(true)
}
observed.forEach((value: any) => {
wrapped.forEach((value: any) => {
expect(isReadonly(value)).toBe(true)
})
for (const value of observed.values()) {
for (const value of wrapped.values()) {
expect(isReadonly(value)).toBe(true)
}
for (const [v1, v2] of observed.entries()) {
for (const [v1, v2] of wrapped.entries()) {
expect(isReadonly(v1)).toBe(true)
expect(isReadonly(v2)).toBe(true)
}
Expand Down Expand Up @@ -299,6 +304,9 @@ describe('reactivity/readonly', () => {
test('readonly should track and trigger if wrapping reactive original', () => {
const a = reactive({ n: 1 })
const b = readonly(a)
// should return true since it's wrapping a reactive source
expect(isReactive(b)).toBe(true)

let dummy
effect(() => {
dummy = b.n
Expand All @@ -309,26 +317,26 @@ describe('reactivity/readonly', () => {
expect(dummy).toBe(2)
})

test('observing already observed value should return same Proxy', () => {
test('wrapping already wrapped value should return same Proxy', () => {
const original = { foo: 1 }
const observed = readonly(original)
const observed2 = readonly(observed)
expect(observed2).toBe(observed)
const wrapped = readonly(original)
const wrapped2 = readonly(wrapped)
expect(wrapped2).toBe(wrapped)
})

test('observing the same value multiple times should return same Proxy', () => {
test('wrapping the same value multiple times should return same Proxy', () => {
const original = { foo: 1 }
const observed = readonly(original)
const observed2 = readonly(original)
expect(observed2).toBe(observed)
const wrapped = readonly(original)
const wrapped2 = readonly(original)
expect(wrapped2).toBe(wrapped)
})

test('markNonReactive', () => {
test('markRaw', () => {
const obj = readonly({
foo: { a: 1 },
bar: markNonReactive({ b: 2 })
bar: markRaw({ b: 2 })
})
expect(isReactive(obj.foo)).toBe(true)
expect(isReadonly(obj.foo)).toBe(true)
expect(isReactive(obj.bar)).toBe(false)
})

Expand Down
Loading

0 comments on commit 09b4202

Please sign in to comment.