DerYeger / yeger

Monorepo for @yeger/ NPM packages
MIT License
306 stars 23 forks source link

bug: Duplicated items when `items` changes dynamically #294

Closed dword-design closed 2 months ago

dword-design commented 2 months ago

Affected Packages

Description

When the items prop is a dynamic array which changes, there are duplicate items. Reason is probably that the watcher does not watch () => props.items but an array containing items. But items isn't necessarily a reactive variable. Storing the array in a computed property fixes the issue.

Reproduction

Pass an array where new items are added depending on a variable.

Additional context

No response

Preferences

DerYeger commented 2 months ago

Watching an array of refs is valid reactivity, see https://vuejs.org/guide/essentials/watchers#watch-source-types.

The docs of this library describe how to add items correctly.

dword-design commented 2 months ago

@DerYeger Yes but this only works if the vars watched in the array are reactive (ref, reactive, computed). A prop needs to be watched via a function call. See also here.

DerYeger commented 2 months ago

@DerYeger Yes but this only works if the vars watched in the array are reactive (ref, reactive, computed). A prop needs to be watched via a function call. See also here.

Indeed, hence items is a ref created via toRefs(props). I'm not using a deep watcher on purpose to prevent performance issues with large items arrays.

dword-design commented 2 months ago

hmmm ok I'm not sure if this is intended. If it's a decision I think it should be added to the docs since it's not obvious and contradicts to Vue intuition. I guess it's related to the comment about push.

DerYeger commented 2 months ago

hmmm ok I'm not sure if this is intended. If it's a decision I think it should be added to the docs since it's not obvious and contradicts to Vue intuition. I guess it's related to the comment about push.

Yes, it is intended and already mentioned in the docs: "To add new items, assign a new value to the items property, e.g., items.value = [...items.value, newItem]. DO NOT push items to the array (e.g., items.value.push(newItem)), as such mutations will not be detected by the reactivity."

dword-design commented 2 months ago

That's not exactly what the bug describes. The paragraph is about push, a specific function that changes the array. The bug is about every deep change to the array, so e.g. passing it directly as a prop is a different case. So it describes a sub-case but not the root issue.