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

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

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

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

I think that happens because of this code here:

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
wangdaoo pushed a commit to wangdaoo/core that referenced this issue May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants