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

v-memo on child of v-for does not render or updates properly #4262

Open
lidlanca opened this issue Aug 5, 2021 · 6 comments
Open

v-memo on child of v-for does not render or updates properly #4262

lidlanca opened this issue Aug 5, 2021 · 6 comments
Labels
🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. ✨ feature request New feature or request

Comments

@lidlanca
Copy link
Contributor

lidlanca commented Aug 5, 2021

Version

3.2.0-beta.7

Reproduction link

sfc playground repro

Steps to reproduce

open the Reproduction link
click on inc button, every 5 clicks the memo should trigger an update

there are 2 lists.

  • the first one the v-memo is on the child element
  • the 2nd list the v-memo is on the element with the v-for.

in the second list all items will update when the memo dependency changes.
in the first list, there are 2 problems.

  1. only the last item gets updated.
  2. the props passed to the component all render as 0 in all items. ( except for odd values on the last item in the list)

What is expected?

2nd list should probably render the same as 1st list

What is actually happening?

inconsistent render with missing values of props

@posva
Copy link
Member

posva commented Aug 5, 2021

This is not correct usage:

  • flip is not even reactive
  • flip isn't used in the subtree where memo is placed

It's great you're playing around with the API and trying to report back bugs but as said in the PR, if you think memo could be used in other scenarios, described those scenarios first instead of showing abstract cases that are not realistic and are not what memo is trying to solve either

@posva posva closed this as completed Aug 5, 2021
@lidlanca
Copy link
Contributor Author

lidlanca commented Aug 5, 2021

in the example the count increment is what triggers the component update.

forcing the reevaluation of the memo to check if the value changes since last render

https://github.com/vuejs/docs/blob/55505e758466a43e7e9c6e406dc2be4632604f71/src/api/directives.md#v-memo-

Memoize a sub-tree of the template. Can be used on both elements and components.
The directive expects a fixed-length array of dependency values to compare for the memoization.
If every value in the array was the same as last render, then updates for the entire sub-tree will be skipped. For example:

  • no where in the docs it is indicated that flip has to be reactive.
  • using flip in the subtree reactive or not makes no difference

makes no difference

also a minimal reproduction of a problem should not reflect a business problem or a use case, it sufficient to show that there is a problem in the most simplistic way for developers to find or identify the bug.

there are 2 v-for list that seemingly should have behaved the same and they are not, this is indicative of a bug.

@posva
Copy link
Member

posva commented Aug 9, 2021

Thanks for the repro. I actually misread this issue, the different behavior is indeed a bug.

no where in the docs it is indicated that flip has to be reactive

There are: non-reactive variable changes don't trigger template updates in general. In this case, it makes no difference because other variables are triggering a template update but if there are none, you won't see any updates (click 5 times to see no changes)

@posva posva reopened this Aug 9, 2021
@posva posva added 🐞 bug Something isn't working ❗ p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. labels Aug 9, 2021
@edison1105
Copy link
Member

(
+ because we disable tracking the block tree here.
_openBlock(true), 
_createElementBlock(_Fragment, null, _renderList(_unref(items), (id, index, ___, _cached) => {
      const _memo = ([ _unref(flip) ])
+ here currentBlock is null, As a result, the block tree is not tracked correncly in isMemoSame.
      if (_cached && _cached.key === id && _isMemoSame(_cached, _memo)) return _cached
      const _item = (_openBlock(), _createBlock(Item, {
        count: count.value,
        index: index,
        key: id
      }, null, 8 /* PROPS */, ["count", "index"]))
      _item.memo = _memo
      return _item
    }, _cache, 1), 128 /* KEYED_FRAGMENT */))

@yyx990803
Copy link
Member

yyx990803 commented Aug 9, 2021

Ok, this is not a bug, but this is currently by design: v-memo does not work inside v-for, it can only be used with v-for (on the same element). Could probably be improved though.

@yyx990803 yyx990803 added 🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. ✨ feature request New feature or request and removed 🐞 bug Something isn't working ❗ p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. labels Aug 9, 2021
@yeahnoob
Copy link

yeahnoob commented Nov 2, 2021

From the user's point, maybe add this into syntax/context checker of Vue compiler?

As mentioned above, for current state, v-memo cannot be inside v-for".

Update:
Ops.. I just find out that eslint-plugin-vue can check this issue, vue/valid-v-memo

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. ✨ feature request New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants