Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(reactivity): handle objects with custom Symbol.toStringTag #12832

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ddoemonn
Copy link

@ddoemonn ddoemonn commented Feb 9, 2025

close #10483

Problem

When an object from a third-party library defines a custom Symbol.toStringTag, the result of calling Object.prototype.toString.call() on that object becomes something like "[object Goat]" instead of the default "[object Object]". Vue’s reactivity system relies on the result of this call (via its internal toRawType() helper) to determine whether an object is plain and should be wrapped in a reactive proxy. As a consequence, objects with a custom tag are mistakenly treated as non-reactive, and mutations to them do not trigger reactive updates.

Solution

This PR addresses the issue by modifying the internal getTargetType function to include an additional check based on the object's prototype. Specifically:

  • A helper function isPlainObject is introduced (or enhanced) to verify whether an object is plain by checking that its prototype is either Object.prototype or null.
  • In the getTargetType function, if the initial type mapping returns TargetType.INVALID (due to a non-standard tag), the function then checks if the object is plain. If it is, the target type is forced to TargetType.COMMON, ensuring the object is wrapped in a reactive proxy.

This change ensures that objects with a custom Symbol.toStringTag are still recognized as plain objects and become reactive as expected.

@edison1105 edison1105 added scope: reactivity 🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. wait changes labels Feb 10, 2025
Copy link

netlify bot commented Feb 10, 2025

Deploy Preview for vue-sfc-playground failed. Why did it fail? →

Name Link
🔨 Latest commit d9444e5
🔍 Latest deploy log https://app.netlify.com/sites/vue-sfc-playground/deploys/67a9a9ec0045b10009b3acc7

Copy link

netlify bot commented Feb 10, 2025

Deploy Preview for vue-next-template-explorer failed. Why did it fail? →

Name Link
🔨 Latest commit d9444e5
🔍 Latest deploy log https://app.netlify.com/sites/vue-next-template-explorer/deploys/67a9a9ec2c20c10008d52358

let type = targetTypeMap(toRawType(value))

// If the raw type mapping fails, we add extra checks:
if (type === TargetType.INVALID) {
Copy link
Member

@edison1105 edison1105 Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the changes should be placed here

return TargetType.INVALID

rather than in getTargetType

should also consider

class MyArray extends Array {
      get [Symbol.toStringTag]() {
        return 'MyArray'
      }
    }

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@edison1105 I moved the extra type checks into targetTypeMap as suggested and added tests for MyArray. Let me know if you have any more thoughts.

Copy link
Member

@edison1105 edison1105 Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made minor tweaks, see 853e933

LGTM. Thanks~

@edison1105 edison1105 added ready to merge The PR is ready to be merged. and removed wait changes labels Feb 11, 2025
Copy link

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 101 kB (+241 B) 38.2 kB (+70 B) 34.4 kB (+79 B)
vue.global.prod.js 159 kB (+241 B) 58.1 kB (+72 B) 51.7 kB (+59 B)

Usages

Name Size Gzip Brotli
createApp (CAPI only) 46.8 kB (+241 B) 18.3 kB (+62 B) 16.7 kB (+56 B)
createApp 54.8 kB (+241 B) 21.4 kB (+76 B) 19.5 kB (+68 B)
createSSRApp 59 kB (+243 B) 23.1 kB (+72 B) 21 kB (+62 B)
defineCustomElement 59.6 kB (+245 B) 23 kB (+63 B) 20.9 kB (+40 B)
overall 68.8 kB (+241 B) 26.5 kB (+64 B) 24.1 kB (+70 B)

Copy link

pkg-pr-new bot commented Feb 11, 2025

Open in Stackblitz

@vue/compiler-core

npm i https://pkg.pr.new/@vue/compiler-core@12832

@vue/compiler-sfc

npm i https://pkg.pr.new/@vue/compiler-sfc@12832

@vue/compiler-dom

npm i https://pkg.pr.new/@vue/compiler-dom@12832

@vue/compiler-ssr

npm i https://pkg.pr.new/@vue/compiler-ssr@12832

@vue/runtime-core

npm i https://pkg.pr.new/@vue/runtime-core@12832

@vue/runtime-dom

npm i https://pkg.pr.new/@vue/runtime-dom@12832

@vue/reactivity

npm i https://pkg.pr.new/@vue/reactivity@12832

@vue/server-renderer

npm i https://pkg.pr.new/@vue/server-renderer@12832

@vue/shared

npm i https://pkg.pr.new/@vue/shared@12832

vue

npm i https://pkg.pr.new/vue@12832

@vue/compat

npm i https://pkg.pr.new/@vue/compat@12832

commit: 853e933

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. ready to merge The PR is ready to be merged. scope: reactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Symbol.toStringTag stops reactive() and ref() from being reactive.
2 participants