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

Pitfalls of Vue dependency detection may cause redundant dependencies #7573

Closed
xiaodemen opened this issue Jan 31, 2018 · 9 comments · Fixed by #7596
Closed

Pitfalls of Vue dependency detection may cause redundant dependencies #7573

xiaodemen opened this issue Jan 31, 2018 · 9 comments · Fixed by #7596
Labels
bug

Comments

@xiaodemen
Copy link
Contributor

Version

2.4.2

Reproduction link

https://jsfiddle.net/sbmLobvr/9/

Steps to reproduce

click the button，then App component will update twice！

What is expected?

App component updates only once

What is actually happening?

App component updates twice

When props used in child component initialization，they will be detected as dependencies of parent component, Is there possible to stop dependency detection when entering child component initialization。
@LinusBorg
Copy link
Member

Hm, that's indeed odd.

For completeness, here's a version not using the prop in data - it only updates once: https://jsfiddle.net/sbmLobvr/10/

@LinusBorg LinusBorg added the bug label Jan 31, 2018
@posva
Copy link
Member

posva commented Jan 31, 2018

@LinusBorg I don't think there's anything wrong going on: the msg prop is used in the template of the root instance, so it's normal for it to render again if it changes (no matter where the update comes from)

@LinusBorg
Copy link
Member

Yeah, it's ok to update once, but twice in a row doesn't make sense to me.

@LinusBorg
Copy link
Member

Especially since it only updates once if we don't access the prop in data() in the child (see my fiddle)

@posva
Copy link
Member

posva commented Jan 31, 2018 •
edited

Mmh, at some point it only showed up once or maybe I just didn't see it correctly, my bad 😆

@javoski javoski mentioned this issue Feb 2, 2018
Merged
12 tasks
@xiaodemen
Copy link
Contributor Author

@javoski I don‘t think this would be the best solution，not just data option，but the whole child component initialization.
And I think it should be reactive, but as dependency of child component ?

@javoski
Copy link
Member

javoski commented Feb 9, 2018

@xiaodemen Sorry about that there's an ambiguity in the PR's title, component props are reactive, but they shouldn't collect dependencies during initialization of data option.

@xiaodemen
Copy link
Contributor Author

xiaodemen commented Feb 11, 2018 •
edited

@javoski I know, but data option is NOT the only case that shouldn't collect dependencies, e.g. component created hook, before mount hook and immediate watch ... so many other cases you should think about, Is it better to pause the dependency collection then restart it in the perspective of lifecycle, rather than simply reseting Dep.target on each case occuring?

@javoski
Copy link
Member

javoski commented Feb 11, 2018

@xiaodemen Yes, you are right, didn't aware of that, and will try to figure out a better solution.

yyx990803 pushed a commit that referenced this issue Mar 9, 2018
@javoski @yyx990803
fix(core): disable dependency collection in lifecycle hooks and data …
318f29f 
…getter (#7596)

This fixes the parent being updated more than necessary due to collecting child props
as dependencies during its own update computation.

fix #7573
This was referenced Mar 14, 2018
Closed
Merged
Merged
Open
Open
Open
Open
Closed
Merged
Closed
Closed
Merged
Merged
Merged
f2009 pushed a commit to f2009/vue that referenced this issue Jan 25, 2019
@javoski
fix(core): disable dependency collection in lifecycle hooks and data …
7f09eef 
…getter (vuejs#7596)

This fixes the parent being updated more than necessary due to collecting child props
as dependencies during its own update computation.

fix vuejs#7573
@tim-janik tim-janik mentioned this issue Aug 23, 2019
Closed
tim-janik added a commit to tim-janik/beast that referenced this issue Sep 2, 2019
@tim-janik
EBEAST: utilities.js: track this.dom_update() calls reactively
0d78674 
Since vuejs/vue#7573, Vue only tracks data
dependencies during its VNode render() function which is unsuitable
for drawing into DOM nodes (e.g. subsequent width/height patching
by Vue will re-erase <canvas/> elements).

The `dom_updates` Mixin now calls `this.dom_update()` for reliable
rendering into DOM elements, *after* Vue has patched the DOM tree,
and tracks dependencies during synchronous calls.

Signed-off-by: Tim Janik <timj@gnu.org>
tim-janik added a commit to tim-janik/beast that referenced this issue Sep 2, 2019
@tim-janik
Merge branch 'observable_from_getters'
9bc8d41 
* observable_from_getters:
  EBEAST: b/part-thumb.vue: freeze immutable allnotes, no need to observe
  EBEAST: utilities.js: resize_canvas: return the devicepixelratio
  EBEAST: b/piano-roll.vue: swap <style/> and <template/> sections
  EBEAST: b/piano-roll.vue: move to await and observable_from_getters()
	Use observable_from_getters() to track all data that requires `await`
	and use dom_update() to handle dependency tracking and reliable DOM
	canvas redraws.
  BSE: bsepart.cc: emit notify:last_tick, noteschanged, linkschanged events
  EBEAST: b/part-thumb.vue: move to observable_from_getters()
  EBEAST: vue_observable_from_getters(): gracefully handle missing getter
  EBEAST: b/track-view.vue: use dom_animate_playback()
  EBEAST: b/track-list.vue: use dom_animate_playback()
  EBEAST: utilities.js: add dom_animate_playback() to vue_mixins.dom_updates
	This allowes Vue components with the dom_updates mixin to enable/disable
	calls to dom_animate_playback (active) via dom_trigger_animate_playback().
  EBEAST: vue_observable_from_getters: only call getter if !!predicate
  EBEAST: b/track-list.vue: use Vue.observable_from_getters() for Bse.Song
  EBEAST: utilities.js: re-add async-warning for dom_update
  EBEAST: b/track-list.vue: remove unused attr
  EBEAST: b/track-view.vue: use Vue.observable_from_getters() for Bse.Track
  EBEAST: provide Vue.observable_from_getters() for async getters with signals
  EBEAST: return `disconnect()` function from Bse.ObjectIface.on()
  EBEAST: b/track-view.vue: use explicit setup/cleanup code for async track API
  EBEAST: b/track-list.vue: properly handle await queries and cleanups
  EBEAST: utilities.js: add support for `priv_tmpl` to vue_mixins.data_tmpl
  EBEAST: utilities.js: add copy_recursively() for Arrays and simple Objects
  EBEAST: utilities.js: add equals_recursively()
  EBEAST: utilities.js: add discard_remote() stub
  EBEAST: utilities.js: track this.dom_update() calls reactively
	Since vuejs/vue#7573, Vue only tracks data
	dependencies during its VNode render() function which is unsuitable
	for drawing into DOM nodes (e.g. subsequent width/height patching
	by Vue will re-erase <canvas/> elements).
	The `dom_updates` Mixin now calls `this.dom_update()` for reliable
	rendering into DOM elements, *after* Vue has patched the DOM tree,
	and tracks dependencies during synchronous calls.

Signed-off-by: Tim Janik <timj@gnu.org>
@sl1673495 sl1673495 mentioned this issue Apr 1, 2020
Open
aJean pushed a commit to aJean/vue that referenced this issue Aug 19, 2020
@javoski @aJean
fix(core): disable dependency collection in lifecycle hooks and data …
60c5471 
…getter (vuejs#7596)

This fixes the parent being updated more than necessary due to collecting child props
as dependencies during its own update computation.

fix vuejs#7573
@theydy theydy mentioned this issue Feb 20, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(core): props getter shouldn't collect dependencies during initialization of data option (fix #7573)
4 participants
@posva @LinusBorg @javoski @xiaodemen