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): ensure array extension methods execute correctly #11761

Closed
wants to merge 2 commits into from

Conversation

linzhe141
Copy link
Contributor

@linzhe141 linzhe141 commented Aug 31, 2024

close #11759

this pr -- playground

I reverted these two PRs. I think it's more reasonable to judge whether a method is an extended array method when accessing array methods

[#11574] [#11629 ]

Copy link

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 99.8 kB (-197 B) 37.6 kB (-28 B) 33.9 kB (-72 B)
vue.global.prod.js 158 kB (-197 B) 57.5 kB (-34 B) 51.1 kB (-18 B)

Usages

Name Size Gzip Brotli
createApp 54.9 kB (-197 B) 21.2 kB (-23 B) 19.3 kB (-27 B)
createSSRApp 58.9 kB (-197 B) 22.8 kB (-47 B) 20.8 kB (-40 B)
defineCustomElement 59.6 kB (-197 B) 22.7 kB (-49 B) 20.7 kB (-36 B)
overall 68.6 kB (-197 B) 26.2 kB (-46 B) 23.9 kB (+22 B)

Copy link

pkg-pr-new bot commented Aug 31, 2024

commit: d3bca06

@vue/compiler-core

pnpm add https://pkg.pr.new/@vue/compiler-core@11761

@vue/compiler-dom

pnpm add https://pkg.pr.new/@vue/compiler-dom@11761

@vue/compiler-sfc

pnpm add https://pkg.pr.new/@vue/compiler-sfc@11761

@vue/compiler-ssr

pnpm add https://pkg.pr.new/@vue/compiler-ssr@11761

@vue/reactivity

pnpm add https://pkg.pr.new/@vue/reactivity@11761

@vue/runtime-core

pnpm add https://pkg.pr.new/@vue/runtime-core@11761

@vue/runtime-dom

pnpm add https://pkg.pr.new/@vue/runtime-dom@11761

@vue/server-renderer

pnpm add https://pkg.pr.new/@vue/server-renderer@11761

@vue/shared

pnpm add https://pkg.pr.new/@vue/shared@11761

vue

pnpm add https://pkg.pr.new/vue@11761

@vue/compat

pnpm add https://pkg.pr.new/@vue/compat@11761

Open in Stackblitz

if (
targetIsArray &&
// @ts-expect-error our code is limited to es2016 but user code is not
(target as any[])[key] === Array.prototype[key] &&
Copy link
Member

@edison1105 edison1105 Aug 31, 2024

Choose a reason for hiding this comment

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

This means that extended methods lose tracking optimization

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry, but I don't know what "tracking optimization" means. Is it referring to this part?
image

Copy link
Member

Choose a reason for hiding this comment

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

see #9511

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If that's the case, the extended methods indeed lose tracking optimization.

However, I'm puzzled by the mismatch between the variable values and the function's parameter definition during debugging. Although the problem was solved by using arguments.

image

@edison1105 edison1105 added scope: reactivity ready for review This PR requires more reviews 🐞 bug Something isn't working labels Aug 31, 2024
@yyx990803
Copy link
Member

We do want to apply the same instrumentations on extended arrays.

@yyx990803 yyx990803 closed this Sep 3, 2024
@linzhe141 linzhe141 deleted the fix-array-extend-method branch September 3, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working ready for review This PR requires more reviews scope: reactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modifying items of an extended array does not trigger re-computation
3 participants