Skip to content

Commit

Permalink
fix: improve isReadonly behaviour, close #811, close #812
Browse files Browse the repository at this point in the history
  • Loading branch information
antfu committed Sep 9, 2021
1 parent b625420 commit d3c456a
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 27 deletions.
2 changes: 2 additions & 0 deletions src/reactivity/readonly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ export function readonly<T extends object>(
): DeepReadonly<UnwrapNestedRefs<T>> {
if (__DEV__ && !isObject(target)) {
warn(`value cannot be made reactive: ${String(target)}`)
} else {
readonlySet.set(target, true)
}
return target as any
}
Expand Down
3 changes: 2 additions & 1 deletion src/reactivity/ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ export function createRef<T>(options: RefOption<T>, readonly = false) {
// related issues: #79
const sealed = Object.seal(r)

readonlySet.set(sealed, true)
if (readonly) readonlySet.set(sealed, true)

return sealed
}

Expand Down
72 changes: 46 additions & 26 deletions test/v3/reactivity/readonly.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import {
watch,
nextTick,
readonly,
isReadonly,
toRaw,
computed,
} from '../../../src'

const Vue = require('vue/dist/vue.common.js')
Expand Down Expand Up @@ -292,21 +295,21 @@ describe('reactivity/readonly', () => {
// })
// })

// test('calling reactive on an readonly should return readonly', () => {
// const a = readonly({})
// const b = reactive(a)
// expect(isReadonly(b)).toBe(true)
// // should point to same original
// expect(toRaw(a)).toBe(toRaw(b))
// })
test('calling reactive on an readonly should return readonly', () => {
const a = readonly({})
const b = reactive(a)
expect(isReadonly(b)).toBe(true)
// should point to same original
expect(toRaw(a)).toBe(toRaw(b))
})

// test('calling readonly on a reactive object should return readonly', () => {
// const a = reactive({})
// const b = readonly(a)
// expect(isReadonly(b)).toBe(true)
// // should point to same original
// expect(toRaw(a)).toBe(toRaw(b))
// })
test('calling readonly on a reactive object should return readonly', () => {
const a = reactive({})
const b = readonly(a)
expect(isReadonly(b)).toBe(true)
// should point to same original
expect(toRaw(a)).toBe(toRaw(b))
})

// test('readonly should track and trigger if wrapping reactive original', () => {
// const a = reactive({ n: 1 })
Expand All @@ -324,19 +327,19 @@ describe('reactivity/readonly', () => {
// expect(dummy).toBe(2)
// })

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

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

// test('markRaw', () => {
// const obj = readonly({
Expand Down Expand Up @@ -474,5 +477,22 @@ describe('reactivity/readonly', () => {
).toHaveBeenWarned()
expect(vm.$el.textContent).toBe(`1`)
})

it('should mark computed as readonly', () => {
expect(isReadonly(computed(() => {}))).toBe(true)
expect(
isReadonly(
computed({
get: () => {},
set: () => {},
})
)
).toBe(false)
})

// #811
it('should not mark ref as readonly', () => {
expect(isReadonly(ref([]))).toBe(false)
})
})
})

0 comments on commit d3c456a

Please sign in to comment.