Skip to content

Commit

Permalink
refactor(reactivity): make readonly non-tracking
Browse files Browse the repository at this point in the history
  • Loading branch information
@yyx990803
yyx990803 committed Apr 15, 2020
1 parent 09b44e0 commit 3178504
Show file tree
Hide file tree
Showing 11 changed files with 110 additions and 250 deletions.
129 changes: 15 additions & 114 deletions packages/reactivity/__tests__/readonly.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import {
isReactive,
isReadonly,
markNonReactive,
lock,
unlock,
effect,
ref,
shallowReadonly
Expand Down Expand Up @@ -90,22 +88,7 @@ describe('reactivity/readonly', () => {
).toHaveBeenWarnedLast()
})

it('should allow mutation when unlocked', () => {
const observed: any = readonly({ foo: 1, bar: { baz: 2 } })
unlock()
observed.prop = 2
observed.bar.qux = 3
delete observed.bar.baz
delete observed.foo
lock()
expect(observed.prop).toBe(2)
expect(observed.foo).toBeUndefined()
expect(observed.bar.qux).toBe(3)
expect('baz' in observed.bar).toBe(false)
expect(`target is readonly`).not.toHaveBeenWarned()
})

it('should not trigger effects when locked', () => {
it('should not trigger effects', () => {
const observed: any = readonly({ a: 1 })
let dummy
effect(() => {
Expand All @@ -117,20 +100,6 @@ describe('reactivity/readonly', () => {
expect(dummy).toBe(1)
expect(`target is readonly`).toHaveBeenWarned()
})

it('should trigger effects when unlocked', () => {
const observed: any = readonly({ a: 1 })
let dummy
effect(() => {
dummy = observed.a
})
expect(dummy).toBe(1)
unlock()
observed.a = 2
lock()
expect(observed.a).toBe(2)
expect(dummy).toBe(2)
})
})

describe('Array', () => {
Expand Down Expand Up @@ -182,23 +151,7 @@ describe('reactivity/readonly', () => {
expect(`target is readonly.`).toHaveBeenWarnedTimes(5)
})

it('should allow mutation when unlocked', () => {
const observed: any = readonly([{ foo: 1, bar: { baz: 2 } }])
unlock()
observed[1] = 2
observed.push(3)
observed[0].foo = 2
observed[0].bar.baz = 3
lock()
expect(observed.length).toBe(3)
expect(observed[1]).toBe(2)
expect(observed[2]).toBe(3)
expect(observed[0].foo).toBe(2)
expect(observed[0].bar.baz).toBe(3)
expect(`target is readonly`).not.toHaveBeenWarned()
})

it('should not trigger effects when locked', () => {
it('should not trigger effects', () => {
const observed: any = readonly([{ a: 1 }])
let dummy
effect(() => {
Expand All @@ -214,30 +167,6 @@ describe('reactivity/readonly', () => {
expect(dummy).toBe(1)
expect(`target is readonly`).toHaveBeenWarnedTimes(2)
})

it('should trigger effects when unlocked', () => {
const observed: any = readonly([{ a: 1 }])
let dummy
effect(() => {
dummy = observed[0].a
})
expect(dummy).toBe(1)

unlock()

observed[0].a = 2
expect(observed[0].a).toBe(2)
expect(dummy).toBe(2)

observed[0] = { a: 3 }
expect(observed[0].a).toBe(3)
expect(dummy).toBe(3)

observed.unshift({ a: 4 })
expect(observed[0].a).toBe(4)
expect(dummy).toBe(4)
lock()
})
})

const maps = [Map, WeakMap]
Expand Down Expand Up @@ -275,23 +204,6 @@ describe('reactivity/readonly', () => {
).toHaveBeenWarned()
})

test('should allow mutation & trigger effect when unlocked', () => {
const map = readonly(new Collection())
const isWeak = Collection === WeakMap
const key = {}
let dummy
effect(() => {
dummy = map.get(key) + (isWeak ? 0 : map.size)
})
expect(dummy).toBeNaN()
unlock()
map.set(key, 1)
lock()
expect(dummy).toBe(isWeak ? 1 : 2)
expect(map.get(key)).toBe(1)
expect(`target is readonly`).not.toHaveBeenWarned()
})

if (Collection === Map) {
test('should retrieve readonly values on iteration', () => {
const key1 = {}
Expand Down Expand Up @@ -346,22 +258,6 @@ describe('reactivity/readonly', () => {
).toHaveBeenWarned()
})

test('should allow mutation & trigger effect when unlocked', () => {
const set = readonly(new Collection())
const key = {}
let dummy
effect(() => {
dummy = set.has(key)
})
expect(dummy).toBe(false)
unlock()
set.add(key)
lock()
expect(dummy).toBe(true)
expect(set.has(key)).toBe(true)
expect(`target is readonly`).not.toHaveBeenWarned()
})

if (Collection === Set) {
test('should retrieve readonly values on iteration', () => {
const original = new Collection([{}, {}])
Expand Down Expand Up @@ -400,6 +296,19 @@ describe('reactivity/readonly', () => {
expect(toRaw(a)).toBe(toRaw(b))
})

test('readonly should track and trigger if wrapping reactive original', () => {
const a = reactive({ n: 1 })
const b = readonly(a)
let dummy
effect(() => {
dummy = b.n
})
expect(dummy).toBe(1)
a.n++
expect(b.n).toBe(2)
expect(dummy).toBe(2)
})

test('observing already observed value should return same Proxy', () => {
const original = { foo: 1 }
const observed = readonly(original)
Expand Down Expand Up @@ -458,13 +367,5 @@ describe('reactivity/readonly', () => {
`Set operation on key "foo" failed: target is readonly.`
).not.toHaveBeenWarned()
})

test('should keep reactive properties reactive', () => {
const props: any = shallowReadonly({ n: reactive({ foo: 1 }) })
unlock()
props.n = reactive({ foo: 2 })
lock()
expect(isReactive(props.n)).toBe(true)
})
})
})
82 changes: 36 additions & 46 deletions packages/reactivity/src/baseHandlers.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { reactive, readonly, toRaw } from './reactive'
import { TrackOpTypes, TriggerOpTypes } from './operations'
import { track, trigger, ITERATE_KEY } from './effect'
import { LOCKED } from './lock'
import { isObject, hasOwn, isSymbol, hasChanged, isArray } from '@vue/shared'
import { isRef } from './ref'

Expand All @@ -12,7 +11,7 @@ const builtInSymbols = new Set(
)

const get = /*#__PURE__*/ createGetter()
const shallowReactiveGet = /*#__PURE__*/ createGetter(false, true)
const shallowGet = /*#__PURE__*/ createGetter(false, true)
const readonlyGet = /*#__PURE__*/ createGetter(true)
const shallowReadonlyGet = /*#__PURE__*/ createGetter(true, true)

Expand Down Expand Up @@ -41,57 +40,47 @@ function createGetter(isReadonly = false, shallow = false) {
return Reflect.get(arrayInstrumentations, key, receiver)
}
const res = Reflect.get(target, key, receiver)

if (isSymbol(key) && builtInSymbols.has(key)) {
return res
}

if (shallow) {
track(target, TrackOpTypes.GET, key)
// TODO strict mode that returns a shallow-readonly version of the value
!isReadonly && track(target, TrackOpTypes.GET, key)
return res
}

if (isRef(res)) {
if (targetIsArray) {
track(target, TrackOpTypes.GET, key)
!isReadonly && track(target, TrackOpTypes.GET, key)
return res
} else {
// ref unwrapping, only for Objects, not for Arrays.
return res.value
}
} else {
track(target, TrackOpTypes.GET, key)
return isObject(res)
? isReadonly
? // need to lazy access readonly and reactive here to avoid
// circular dependency
readonly(res)
: reactive(res)
: res
}

!isReadonly && track(target, TrackOpTypes.GET, key)
return isObject(res)
? isReadonly
? // need to lazy access readonly and reactive here to avoid
// circular dependency
readonly(res)
: reactive(res)
: res
}
}

const set = /*#__PURE__*/ createSetter()
const shallowReactiveSet = /*#__PURE__*/ createSetter(false, true)
const readonlySet = /*#__PURE__*/ createSetter(true)
const shallowReadonlySet = /*#__PURE__*/ createSetter(true, true)
const shallowSet = /*#__PURE__*/ createSetter(true)

function createSetter(isReadonly = false, shallow = false) {
function createSetter(shallow = false) {
return function set(
target: object,
key: string | symbol,
value: unknown,
receiver: object
): boolean {
if (isReadonly && LOCKED) {
if (__DEV__) {
console.warn(
`Set operation on key "${String(key)}" failed: target is readonly.`,
target
)
}
return true
}

const oldValue = (target as any)[key]
if (!shallow) {
value = toRaw(value)
Expand Down Expand Up @@ -148,37 +137,38 @@ export const mutableHandlers: ProxyHandler<object> = {

export const readonlyHandlers: ProxyHandler<object> = {
get: readonlyGet,
set: readonlySet,
has,
ownKeys,
deleteProperty(target: object, key: string | symbol): boolean {
if (LOCKED) {
if (__DEV__) {
console.warn(
`Delete operation on key "${String(
key
)}" failed: target is readonly.`,
target
)
}
return true
} else {
return deleteProperty(target, key)
set(target, key) {
if (__DEV__) {
console.warn(
`Set operation on key "${String(key)}" failed: target is readonly.`,
target
)
}
return true
},
deleteProperty(target, key) {
if (__DEV__) {
console.warn(
`Delete operation on key "${String(key)}" failed: target is readonly.`,
target
)
}
return true
}
}

export const shallowReactiveHandlers: ProxyHandler<object> = {
...mutableHandlers,
get: shallowReactiveGet,
set: shallowReactiveSet
get: shallowGet,
set: shallowSet
}

// Props handlers are special in the sense that it should not unwrap top-level
// refs (in order to allow refs to be explicitly passed down), but should
// retain the reactivity of the normal readonly object.
export const shallowReadonlyHandlers: ProxyHandler<object> = {
...readonlyHandlers,
get: shallowReadonlyGet,
set: shallowReadonlySet
get: shallowReadonlyGet
}
Loading

0 comments on commit 3178504

Please sign in to comment.