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.

Sign up for GitHub

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

Jump to bottom

Template ref array is updated twice when an array ref dependency is changed #9908

Open
abu-co opened this issue Dec 24, 2023 · 5 comments
Open

Template ref array is updated twice when an array ref dependency is changed #9908

abu-co opened this issue Dec 24, 2023 · 5 comments
Labels
🔩 p2-edge-case

Comments

@abu-co
Copy link

abu-co commented Dec 24, 2023

Vue version

3.3.13

Link to minimal reproduction

https://play.vuejs.org/#eNp9Ul1P20AQ/CurE1Ic4Tpt0/YhcoxaFImiFqqSt1wkXGcdDuw76z7cIMv/nT1/hDwg3nwzs vZ2W3Y96qKaodswWKTaVFZMGhdBUUq90vOrOEs4VKUldIWGshUWTmLuxA05tBCrlUJE2ow4ZLLTEljQbryH2oDS68JNp/Cz F8Ox3ZB2EvlZN2oD8SMVJYYInSDpXx1fr3r1UPbbZJsHntkZFidBIEU1gm0HAJx95RnRYOz889pmkcLeH rMl6GC4uYDJpT3521lzf3d5Exmoh9yJ/DkauL5i2nMt7LtvOKsWzFiUqZ4c/D MOzZew RJ Db9tQ/CjxbM VcqQHhbLqkgt0gsgrjQmDUUKbRvP/KNHkytBEw4REX8MrJP1op2oof6QK00rkiDkaIIznxyB4wQELJ7w2ctokdRNdm2onhrFsxNDLKRlU7y52EePRkm6iC5TznzSokB9W1lB8XO26NP2XFoU6v91h1ntMBzx7AGzpzfwR3PwGGd/NBrUNXJ25Gyq92h7enV3gwf6PpKl2rmC1O Qf9GownmPveyHkzuyfaLr3P7sbpk2vTarg0VpxqG8Ua9sOz1ndNWX74z ancezbs6OhHWvgBYwByK

Steps to reproduce

Please observe the output from the computed property c.

What is expected?

elements should only update once when numbers is updated.

What is actually happening?

elements is first set to [] before it is populated, causing any dependent computed properties to be evaluated twice.

System Info

No response

Any additional comments?

Perhaps this behaviour is intentional? 🤔
@edison1105
Copy link
Member

edison1105 commented Dec 25, 2023 •
edited
Loading

fixed by #5912 (in version 3.4)

Switch to version 3.4 DEV mode will get a maximum recursive error.
/cc @johnsoncodehk

I made a pr to fix this, see #9920

@edison1105
Copy link
Member

@abu-co
You should avoid using self-referencing in the computed. it will get an error in version 3.4.
see more #9920 (comment)

@abu-co
Copy link
Author

abu-co commented Dec 26, 2023

You should avoid using self-referencing in the computed. it will get an error in version 3.4.

@edison1105 Noted! Thanks for pointing that out.

fixed by #5912 (in version 3.4)

Hmm... the computed property still appears to have been evaluated twice when a single modification was made to its dependency in the latest version of Vue, though?

Please have a look at this further simplified demo, which uses the version containing the latest commit 0695c69: https://play.vuejs.org/#__DEV__eNp9Ul1v00AQ/CurU6UmqnEKAR6CEwmqSFBBi2jecpFqnHV6rb1n3YdJZfm/s2fHaR6qvnlnZ dmZ92Ir1UV1x7FTCQ2M6pyYNH5CoqUdnMpnJViIUmVlTYOGsh0WXmH2wgM5tBCbnQJ5yxwLklSpsk6IF/ RWNhHjij9fvoQzTdjCUV6OBBuSvtyXHz8mUCCyyR3GEk b769XPZQ vNYrQOwz0xY8ZgYTQaw3wBjSQ4yl5chMrwBobg/kT2rLm u72JrTOKdip/Hg29uE4Lj MWpKQTc2fNULT3klo2IImDWakStXeHpw L9hI8tP4YfYo byK4HH RlEz6QDk LhyWVZE65AogqQwuGk4T2jaZhKJDt6qG l2uDQdPoGh4QIoQC4ODaQZmT/gcaHweFqJOiOdZKJmcvCUiPiFnl6td/Gg18Z27wKQIMaoCzW3lFGcrxayPMvTSotD/rjvMGY/RgGcPmD29gj/afcCk G3QoqlRimPPpWaHrm8v725wz9/HZqm3vmD2G80/aHXhg8ee9s3Tlm2f8Dq3P7o/lI 7ssu9Q7LDUsFoYLYdXwr V6/eWP3F7jSednN8ftH B5gJDxM=

The hitCount is incremented every time c is evaluated.

I'm not really familiar with the internal mechanisms involved here, but I'm guessing this is what is happening:

  1. When elements is initialized to [], c is evaluated for the first time;

  2. When elements is populated with the appropriate references to the elements after render, c is evaluated for the second time;

  3. When numbers is set to a completely different array:

    • elements is first set to [], causing c to be evaluated for the third time (which is unexpected and may cause overhead);
    • elements is then populated with references to the new elements, causing c to be evaluated for the fourth time.

    ... So c is unnecessarily evaluated twice here, despite only one modification being made?

@LinusBorg
Copy link
Member

LinusBorg commented Dec 26, 2023 •
edited
Loading

I think that happens because of this code here:

core/packages/runtime-core/src/rendererTemplateRef.ts

Lines 121 to 129 in 947aacf

if (value) {
// #1789: for non-null values, set them after render
// null values means this is unmount and it should not overwrite another
// ref with the same key
;(doSet as SchedulerJob).id = -1
queuePostRenderEffect(doSet, parentSuspense)
} else {
doSet()
}

  • When elements are removed from the template ref array, that happens synchronously
  • Which means the computed is re-evaluated in the current scheduler flush
  • All additions of new elements to the elements array are done as a post render effect,
  • and that triggers the second evaluation of the computed.

From the comments in the source, we can see this was done to solve #1789 so this might be necessary even though it can lead to some double triggers, essentially.

@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 2024
@yyx990803 yyx990803 reopened this Feb 7, 2024
@vuejs vuejs unlocked this conversation Feb 7, 2024
@yyx990803
Copy link
Member

Change in de4d2e2 led to regressions (#10210, #10234) so it is reverted for now.

huiliangShen pushed a commit to huiliangShen/core that referenced this issue Feb 7, 2024
@yyx990803
revert: perf(templateRef): avoid double render when using template re…
eb1b911 
…f on v-for

This reverts commit de4d2e2.

reopen vuejs#9908
close vuejs#10210, close vuejs#10234
@dylankelly dylankelly mentioned this issue Feb 20, 2024
Closed
lynxlangya pushed a commit to lynxlangya/core that referenced this issue May 30, 2024
@yyx990803 @lynxlangya
revert: perf(templateRef): avoid double render when using template re…
5f4d413 
…f on v-for

This reverts commit de4d2e2.

reopen vuejs#9908
close vuejs#10210, close vuejs#10234
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
🔩 p2-edge-case
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@yyx990803 @LinusBorg @edison1105 @abu-co