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

Non-atomic prop updates #1763

Closed
skirtles-code opened this issue Aug 3, 2020 · 1 comment
Closed

Non-atomic prop updates #1763

skirtles-code opened this issue Aug 3, 2020 · 1 comment

Comments

@skirtles-code
Copy link
Contributor

Version

3.0.0-rc.5

Reproduction link

https://jsfiddle.net/skirtle/9ud26a4m/50/

Steps to reproduce

  1. Clicking the button changes optionsIndex, which updates two props on the child.
  2. The child's computed property path is calculated using these two props.
  3. Notice in the console logging that the path property temporarily has an inconsistent value where it uses one of the new prop values and one of the old prop values.

What is actually happening?

The problem is that behind the scenes the child props are updated one at a time and a sync watcher fires immediately, before the other props have been updated.

As a related side note, also notice that the render function gets called twice during update.

What is expected?

I think it would be preferable for updating props to be treated as an atomic transaction, so that they all update before any watchers are triggered.


To preempt some questions/objections/thoughts you may have...

Why do you think this is a bug?

Conceptually the props change in unison, when the parent renders. Updating them one-by-one is an internal implementation detail that shouldn't be exposed via any part of the public API.

Couldn't that be by design?

Maybe. I've not been able to find any public discussion or documentation on this aspect of flush timing. It feels like a major caveat on using flush: 'sync'.

Why are you using flush: 'sync'?

I'm trying to recreate Vue 2 behaviour, where a watch would run before rendering. Using flush: 'pre' won't help when watching
a prop.

Why do you want to run a watcher before rendering?

I've provided an explanation in a comment here. Using a computed property instead is not a viable alternative.

Are atomic prop updates even possible?

I believe so, at least from the perspective of what can be observed via the public API.

I think the most difficult part would be handling cases with multiple computed properties. Computed properties that depend on any of the updated props would need to have their cached values invalidated before any other sync watchers are allowed to run.

Why does it render twice?

I did a small amount of debugging on that. I have a theory...

The props are updated by the parent's patch. The child component is not in the flush queue at that point, so the attempt to invalidate its update job won't do anything.

As the first prop is updated the watcher fires and accesses this.path. That causes path to be added as a rendering dependency, even though it isn't used in the render function (which hasn't been called yet).

When the second prop is updated it also updates the computed property. This triggers the path rendering dependency, so the component gets added to the flush queue.

It's at this point that the first call to render occurs, as part of the parent's patch.

The second render call comes from the entry in the queue.

@yyx990803
Copy link
Member

This is expected behavior for sync watcher, since atomic updates is what the scheduler is responsible for (and sync watchers are fired without being scheduled).

However flush: 'pre' watchers should fire before the component updates, so that is fixed in d4c17fb.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants