Skip to content

Commit

Permalink
fix(reactivity): avoid using WeakMap for IE compatibility
Browse files Browse the repository at this point in the history
Using a WeakMap polyfill isn't ideal because the reason we tried to use
WeakMap was to work with non-extensible objects. However, WeakMap
polyfill for non-extensible objects are non-weak and could lead to
memory leaks.

The trade-off is that we remove support for `readonly()` on
non-extensible objects, which seems reasonable.

close #12837
  • Loading branch information
yyx990803 committed Nov 9, 2022
1 parent d1899ca commit 29b5f58
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 18 deletions.
2 changes: 0 additions & 2 deletions src/core/observer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
noop
} from '../util/index'
import { isReadonly, isRef, TrackOpTypes, TriggerOpTypes } from '../../v3'
import { rawMap } from '../../v3/reactivity/reactive'

const arrayKeys = Object.getOwnPropertyNames(arrayMethods)

Expand Down Expand Up @@ -116,7 +115,6 @@ export function observe(
(isArray(value) || isPlainObject(value)) &&
Object.isExtensible(value) &&
!value.__v_skip /* ReactiveFlags.SKIP */ &&
!rawMap.has(value) &&
!isRef(value) &&
!(value instanceof VNode)
) {
Expand Down
10 changes: 4 additions & 6 deletions src/v3/reactivity/reactive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,10 @@ import {
isPrimitive,
warn,
toRawType,
isServerRendering,
isObject
isServerRendering
} from 'core/util'
import type { Ref, UnwrapRefSimple, RawSymbol } from './ref'

export const rawMap = new WeakMap()

export const enum ReactiveFlags {
SKIP = '__v_skip',
IS_READONLY = '__v_isReadonly',
Expand Down Expand Up @@ -122,8 +119,9 @@ export function toRaw<T>(observed: T): T {
export function markRaw<T extends object>(
value: T
): T & { [RawSymbol]?: true } {
if (isObject(value)) {
rawMap.set(value, true)
// non-extensible objects won't be observed anyway
if (Object.isExtensible(value)) {
def(value, ReactiveFlags.SKIP, true)
}
return value
}
Expand Down
16 changes: 11 additions & 5 deletions src/v3/reactivity/readonly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ export type DeepReadonly<T> = T extends Builtin
? { readonly [K in keyof T]: DeepReadonly<T[K]> }
: Readonly<T>

const rawToReadonlyMap = new WeakMap()
const rawToShallowReadonlyMap = new WeakMap()
const rawToReadonlyFlag = `__v_rawToReadonly`
const rawToShallowReadonlyFlag = `__v_rawToShallowReadonly`

export function readonly<T extends object>(
target: T
Expand All @@ -57,20 +57,26 @@ function createReadonly(target: any, shallow: boolean) {
return target as any
}

if (__DEV__ && !Object.isExtensible(target)) {
warn(
`Vue 2 does not support creating readonly proxy for non-extensible object.`
)
}

// already a readonly object
if (isReadonly(target)) {
return target as any
}

// already has a readonly proxy
const map = shallow ? rawToShallowReadonlyMap : rawToReadonlyMap
const existingProxy = map.get(target)
const existingFlag = shallow ? rawToShallowReadonlyFlag : rawToReadonlyFlag
const existingProxy = target[existingFlag]
if (existingProxy) {
return existingProxy
}

const proxy = Object.create(Object.getPrototypeOf(target))
map.set(target, proxy)
def(target, existingFlag, proxy)

def(proxy, ReactiveFlags.IS_READONLY, true)
def(proxy, ReactiveFlags.RAW, target)
Expand Down
2 changes: 1 addition & 1 deletion test/unit/features/v3/reactivity/reactive.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ describe('reactivity/reactive', () => {
})

test('markRaw on non-extensible objects', () => {
const foo = Object.freeze({})
const foo = Object.seal({})
markRaw(foo)
expect(isReactive(reactive(foo))).toBe(false)
})
Expand Down
11 changes: 7 additions & 4 deletions test/unit/features/v3/reactivity/readonly.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -526,10 +526,13 @@ describe('reactivity/readonly', () => {
expect(`et operation on key "x" failed`).toHaveBeenWarned()
})

test('compatibility with non-extensible objects', () => {
test('warn non-extensible objects', () => {
const foo = Object.freeze({ a: 1 })
const bar = readonly(foo)
expect(isReadonly(bar)).toBe(true)
expect(readonly(foo)).toBe(bar)
try {
readonly(foo)
} catch (e) {}
expect(
`Vue 2 does not support creating readonly proxy for non-extensible object`
).toHaveBeenWarned()
})
})

0 comments on commit 29b5f58

Please sign in to comment.