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(watch): deep watching symbol properties #10969

Merged
merged 1 commit into from
May 30, 2024

Conversation

skirtles-code
Copy link
Contributor

Currently, deep watchers don't traverse symbol properties.

#402 already attempts a fix for this, but that seems to have stalled due to some performance tweaks that were made at the same time. Here I've just tried to fix the core problem with symbols.

String properties are traversed using for/in. That traverses enumerable properties, including those inherited from the prototype. I didn't want to do anything that might break that, so I've left it untouched.

For symbols I've used getOwnPropertySymbols and propertyIsEnumerable. This doesn't take the prototype chain into account.

Is traversing symbol properties actually useful? I don't know. If not, perhaps it should be documented that they aren't supported? Currently it's a bit inconsistent, as adding or removing a symbol property will trigger the watcher, but updating the value or mutating a nested value won't.

I've marked this as a fix, though it could be viewed as a feat.

Copy link

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 90.8 kB (+110 B) 34.5 kB (+35 B) 31.2 kB (+99 B)
vue.global.prod.js 148 kB (+110 B) 53.7 kB (+26 B) 48 kB (+1 B)

Usages

Name Size Gzip Brotli
createApp 50.9 kB (+110 B) 19.9 kB (+32 B) 18.1 kB (+36 B)
createSSRApp 54.2 kB (+110 B) 21.2 kB (+30 B) 19.3 kB (+35 B)
defineCustomElement 53.2 kB (+110 B) 20.6 kB (+33 B) 18.8 kB (+36 B)
overall 64.6 kB (+110 B) 24.9 kB (+31 B) 22.6 kB (-12 B)

@yyx990803 yyx990803 merged commit a3e8aaf into vuejs:main May 30, 2024
11 checks passed
yangmingshan added a commit to vue-mini/vue-mini that referenced this pull request Jun 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants