From 29b5f588032600baae9854ac9a4105916a5aa648 Mon Sep 17 00:00:00 2001
From: Evan You <yyx990803@gmail.com>
Date: Wed, 9 Nov 2022 20:33:05 +0800
Subject: [PATCH] fix(reactivity): avoid using WeakMap for IE compatibility

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
---
 src/core/observer/index.ts                       |  2 --
 src/v3/reactivity/reactive.ts                    | 10 ++++------
 src/v3/reactivity/readonly.ts                    | 16 +++++++++++-----
 .../unit/features/v3/reactivity/reactive.spec.ts |  2 +-
 .../unit/features/v3/reactivity/readonly.spec.ts | 11 +++++++----
 5 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/src/core/observer/index.ts b/src/core/observer/index.ts
index 51f928c5c2..ef7296567c 100644
--- a/src/core/observer/index.ts
+++ b/src/core/observer/index.ts
@@ -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)
 
@@ -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)
   ) {
diff --git a/src/v3/reactivity/reactive.ts b/src/v3/reactivity/reactive.ts
index c65f25e67d..da9a1bf0c1 100644
--- a/src/v3/reactivity/reactive.ts
+++ b/src/v3/reactivity/reactive.ts
@@ -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',
@@ -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
 }
diff --git a/src/v3/reactivity/readonly.ts b/src/v3/reactivity/readonly.ts
index c3794bb53b..e3a8393030 100644
--- a/src/v3/reactivity/readonly.ts
+++ b/src/v3/reactivity/readonly.ts
@@ -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
@@ -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)
diff --git a/test/unit/features/v3/reactivity/reactive.spec.ts b/test/unit/features/v3/reactivity/reactive.spec.ts
index a8a30e0353..aa49f103f6 100644
--- a/test/unit/features/v3/reactivity/reactive.spec.ts
+++ b/test/unit/features/v3/reactivity/reactive.spec.ts
@@ -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)
   })
diff --git a/test/unit/features/v3/reactivity/readonly.spec.ts b/test/unit/features/v3/reactivity/readonly.spec.ts
index fab99bbfbc..252fb9884b 100644
--- a/test/unit/features/v3/reactivity/readonly.spec.ts
+++ b/test/unit/features/v3/reactivity/readonly.spec.ts
@@ -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()
   })
 })