From 30d1f32b83e68a4191e1019209450cecc0b2a121 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96zer=20G=C3=B6kalpsezer?= Date: Sun, 9 Feb 2025 17:49:31 +0300 Subject: [PATCH 1/6] fix(reactivity): handle objects with custom Symbol.toStringTag --- .../reactivity/__tests__/reactive.spec.ts | 18 ++++++++++++++ packages/reactivity/src/reactive.ts | 24 ++++++++++++++++--- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/packages/reactivity/__tests__/reactive.spec.ts b/packages/reactivity/__tests__/reactive.spec.ts index aabd95456..fa99bc142 100644 --- a/packages/reactivity/__tests__/reactive.spec.ts +++ b/packages/reactivity/__tests__/reactive.spec.ts @@ -112,6 +112,24 @@ describe('reactivity/reactive', () => { expect(dummy).toBe(false) }) + test('reactive object with custom Symbol.toStringTag triggers reactivity', () => { + const original = { [Symbol.toStringTag]: 'Goat', foo: 1 } + const observed = reactive(original) + + expect(isReactive(observed)).toBe(true) + expect(isProxy(observed)).toBe(true) + + let dummy: number | undefined + effect(() => { + dummy = observed.foo + }) + + expect(dummy).toBe(1) + + observed.foo = 2 + expect(dummy).toBe(2) + }) + test('observed value should proxy mutations to original (Object)', () => { const original: any = { foo: 1 } const observed = reactive(original) diff --git a/packages/reactivity/src/reactive.ts b/packages/reactivity/src/reactive.ts index 729c85496..9a133bb76 100644 --- a/packages/reactivity/src/reactive.ts +++ b/packages/reactivity/src/reactive.ts @@ -55,10 +55,28 @@ function targetTypeMap(rawType: string) { } } +function isPlainObject(value: unknown): value is object { + return ( + typeof value === 'object' && + value !== null && + (Object.getPrototypeOf(value) === Object.prototype || + Object.getPrototypeOf(value) === null) + ) +} + function getTargetType(value: Target) { - return value[ReactiveFlags.SKIP] || !Object.isExtensible(value) - ? TargetType.INVALID - : targetTypeMap(toRawType(value)) + if (value[ReactiveFlags.SKIP] || !Object.isExtensible(value)) { + return TargetType.INVALID + } + const rawType = toRawType(value) + let type = targetTypeMap(rawType) + + // If we got INVALID but the value is actually a plain object (even if its raw type was changed + // by a custom Symbol.toStringTag), then force it to be reactive. + if (type === TargetType.INVALID && isPlainObject(value)) { + type = TargetType.COMMON + } + return type } // only unwrap nested ref From d9444e55238abb9a63baf739cdddcc41416b3dc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96zer?= <109994179+ddoemonn@users.noreply.github.com> Date: Mon, 10 Feb 2025 10:25:30 +0300 Subject: [PATCH 2/6] fix: type instead of rawType Co-authored-by: edison --- packages/reactivity/src/reactive.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/reactivity/src/reactive.ts b/packages/reactivity/src/reactive.ts index 9a133bb76..1ac1b68c4 100644 --- a/packages/reactivity/src/reactive.ts +++ b/packages/reactivity/src/reactive.ts @@ -68,7 +68,7 @@ function getTargetType(value: Target) { if (value[ReactiveFlags.SKIP] || !Object.isExtensible(value)) { return TargetType.INVALID } - const rawType = toRawType(value) + const type = targetTypeMap(toRawType(value)) let type = targetTypeMap(rawType) // If we got INVALID but the value is actually a plain object (even if its raw type was changed From b42d264da0ac64da2f8d15b8c346c8d4f61b5042 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96zer=20G=C3=B6kalpsezer?= Date: Mon, 10 Feb 2025 10:41:26 +0300 Subject: [PATCH 3/6] fix(reactivity): support custom Symbol.toStringTag for collections --- .../reactivity/__tests__/reactive.spec.ts | 29 +++++++++++++++++++ packages/reactivity/src/reactive.ts | 24 +++++++++++---- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/packages/reactivity/__tests__/reactive.spec.ts b/packages/reactivity/__tests__/reactive.spec.ts index fa99bc142..710b62c95 100644 --- a/packages/reactivity/__tests__/reactive.spec.ts +++ b/packages/reactivity/__tests__/reactive.spec.ts @@ -130,6 +130,35 @@ describe('reactivity/reactive', () => { expect(dummy).toBe(2) }) + test('custom collection type with custom Symbol.toStringTag is handled as a collection', () => { + class MyCustomMap extends Map { + get [Symbol.toStringTag]() { + return 'MyCustomMap' + } + } + + const myCustomMap = new MyCustomMap() + + expect(Object.prototype.toString.call(myCustomMap)).toBe( + '[object MyCustomMap]', + ) + + const observed = reactive(myCustomMap) + + expect(isReactive(observed)).toBe(true) + expect(isProxy(observed)).toBe(true) + + let dummy: boolean = false + effect(() => { + dummy = observed.has('foo') + }) + + expect(dummy).toBe(false) + + observed.set('foo', 'bar') + expect(dummy).toBe(true) + }) + test('observed value should proxy mutations to original (Object)', () => { const original: any = { foo: 1 } const observed = reactive(original) diff --git a/packages/reactivity/src/reactive.ts b/packages/reactivity/src/reactive.ts index 1ac1b68c4..052fc48ff 100644 --- a/packages/reactivity/src/reactive.ts +++ b/packages/reactivity/src/reactive.ts @@ -68,14 +68,26 @@ function getTargetType(value: Target) { if (value[ReactiveFlags.SKIP] || !Object.isExtensible(value)) { return TargetType.INVALID } - const type = targetTypeMap(toRawType(value)) - let type = targetTypeMap(rawType) - // If we got INVALID but the value is actually a plain object (even if its raw type was changed - // by a custom Symbol.toStringTag), then force it to be reactive. - if (type === TargetType.INVALID && isPlainObject(value)) { - type = TargetType.COMMON + let type = targetTypeMap(toRawType(value)) + + // If the raw type mapping fails, we add extra checks: + if (type === TargetType.INVALID) { + // Check for collection types even if they have a custom Symbol.toStringTag. + if ( + value instanceof Map || + value instanceof Set || + value instanceof WeakMap || + value instanceof WeakSet + ) { + type = TargetType.COLLECTION + } + // Check if the value is a plain object despite a custom tag. + else if (isPlainObject(value)) { + type = TargetType.COMMON + } } + return type } From f65a680b14bba0e1e55cbe91169f1a6fbf2f10a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96zer=20G=C3=B6kalpsezer?= Date: Tue, 11 Feb 2025 11:41:57 +0300 Subject: [PATCH 4/6] fix(reactivity): handle custom Symbol.toStringTag for collections and arrays --- .../reactivity/__tests__/reactive.spec.ts | 27 ++++++++++++++ packages/reactivity/src/reactive.ts | 37 ++++++++----------- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/packages/reactivity/__tests__/reactive.spec.ts b/packages/reactivity/__tests__/reactive.spec.ts index 710b62c95..1ff8bac37 100644 --- a/packages/reactivity/__tests__/reactive.spec.ts +++ b/packages/reactivity/__tests__/reactive.spec.ts @@ -159,6 +159,33 @@ describe('reactivity/reactive', () => { expect(dummy).toBe(true) }) + test('custom array type with custom Symbol.toStringTag is handled as a common object', () => { + class MyArray extends Array { + get [Symbol.toStringTag]() { + return 'MyArray' + } + } + + const myArr = new MyArray() + + expect(Object.prototype.toString.call(myArr)).toBe('[object MyArray]') + + const observed = reactive(myArr) + + expect(isReactive(observed)).toBe(true) + expect(isProxy(observed)).toBe(true) + + let dummy: number = 0 + effect(() => { + dummy = observed.length + }) + + expect(dummy).toBe(0) + + observed.push(42) + expect(dummy).toBe(1) + }) + test('observed value should proxy mutations to original (Object)', () => { const original: any = { foo: 1 } const observed = reactive(original) diff --git a/packages/reactivity/src/reactive.ts b/packages/reactivity/src/reactive.ts index 052fc48ff..517e31151 100644 --- a/packages/reactivity/src/reactive.ts +++ b/packages/reactivity/src/reactive.ts @@ -40,7 +40,7 @@ enum TargetType { COLLECTION = 2, } -function targetTypeMap(rawType: string) { +function targetTypeMap(rawType: string, value: Target): TargetType { switch (rawType) { case 'Object': case 'Array': @@ -51,6 +51,17 @@ function targetTypeMap(rawType: string) { case 'WeakSet': return TargetType.COLLECTION default: + if ( + value instanceof Map || + value instanceof Set || + value instanceof WeakMap || + value instanceof WeakSet + ) { + return TargetType.COLLECTION + } + if (value instanceof Array || isPlainObject(value)) { + return TargetType.COMMON + } return TargetType.INVALID } } @@ -68,27 +79,9 @@ function getTargetType(value: Target) { if (value[ReactiveFlags.SKIP] || !Object.isExtensible(value)) { return TargetType.INVALID } - - let type = targetTypeMap(toRawType(value)) - - // If the raw type mapping fails, we add extra checks: - if (type === TargetType.INVALID) { - // Check for collection types even if they have a custom Symbol.toStringTag. - if ( - value instanceof Map || - value instanceof Set || - value instanceof WeakMap || - value instanceof WeakSet - ) { - type = TargetType.COLLECTION - } - // Check if the value is a plain object despite a custom tag. - else if (isPlainObject(value)) { - type = TargetType.COMMON - } - } - - return type + // Note: We now pass both the raw type and the value to targetTypeMap, + // so that extra instanceof checks can be performed in the default case. + return targetTypeMap(toRawType(value), value) } // only unwrap nested ref From c634448c2ad6b3ade97657486b4a0967cb15b19a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96zer=20G=C3=B6kalpsezer?= Date: Tue, 11 Feb 2025 11:45:15 +0300 Subject: [PATCH 5/6] fix(reactivity): simplify getTargetType function --- packages/reactivity/src/reactive.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/reactivity/src/reactive.ts b/packages/reactivity/src/reactive.ts index 517e31151..bc8cfcf70 100644 --- a/packages/reactivity/src/reactive.ts +++ b/packages/reactivity/src/reactive.ts @@ -76,12 +76,9 @@ function isPlainObject(value: unknown): value is object { } function getTargetType(value: Target) { - if (value[ReactiveFlags.SKIP] || !Object.isExtensible(value)) { - return TargetType.INVALID - } - // Note: We now pass both the raw type and the value to targetTypeMap, - // so that extra instanceof checks can be performed in the default case. - return targetTypeMap(toRawType(value), value) + return value[ReactiveFlags.SKIP] || !Object.isExtensible(value) + ? TargetType.INVALID + : targetTypeMap(toRawType(value), value) } // only unwrap nested ref From 853e9332e6bdaf363fc9d99a1bb21ead427509f0 Mon Sep 17 00:00:00 2001 From: daiwei Date: Tue, 11 Feb 2025 17:02:03 +0800 Subject: [PATCH 6/6] chore: minor tweaks --- packages/reactivity/src/reactive.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/reactivity/src/reactive.ts b/packages/reactivity/src/reactive.ts index bc8cfcf70..f57231816 100644 --- a/packages/reactivity/src/reactive.ts +++ b/packages/reactivity/src/reactive.ts @@ -40,7 +40,8 @@ enum TargetType { COLLECTION = 2, } -function targetTypeMap(rawType: string, value: Target): TargetType { +function targetTypeMap(value: Target): TargetType { + const rawType = toRawType(value) switch (rawType) { case 'Object': case 'Array': @@ -78,7 +79,7 @@ function isPlainObject(value: unknown): value is object { function getTargetType(value: Target) { return value[ReactiveFlags.SKIP] || !Object.isExtensible(value) ? TargetType.INVALID - : targetTypeMap(toRawType(value), value) + : targetTypeMap(value) } // only unwrap nested ref