-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Document why watched template refs require theflush: 'post'
option to be set
#816
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
Document why watched template refs require theflush: 'post'
option to be set
#816
Conversation
flush: 'post'
option to be setflush: 'post'
option to be set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made some small suggestions inline but there's a bigger point I wanted to raise and I wasn't sure where to put it so I've put it here.
I'm sorry for the lengthy essay. I found it really difficult to explain this properly. I hope you can figure out what I mean from my rambling.
In summary, I think the problem reported in vuejs/core#3030 is a bug in Vue. I understand why it looks like user error but I don't think it is.
I think it's important to clarify exactly what's going on here and what the bug is because it impacts how we document this. While there are good reasons why you might want to use flush: 'post'
with template refs, I don't think it's quite as straightforward as it appears.
The description below is somewhat simplified but if I went into all the subtle details I'd be writing it forever.
There are 3 queues within the scheduler, which I'm going to call PRE, RENDER and POST. The queues are processed in that order, with one queue being exhausted before moving on to the next queue:
PRE -> RENDER -> POST
I'm going to refer to this 3-stage process as the 'render cycle'.
When the render cycle completes it checks the queues again. If any new entries have been added to the queues it goes back to the start and does it all again. We can end up with something like this:
PRE -> RENDER -> POST | PRE -> RENDER -> POST | ...
It is worth noting that the POST queue is copied just before it is processed, so any new POST items won't be added to the current POST queue. Instead they'll be added to a new POST queue and will be processed on the next loop around the cycle.
I should also stress that this is a distinct concept from nextTick
. The loop I've described above all happens within the same 'tick'. The 'next tick' is queued on a microtask and only occurs once this loop is complete.
The ref
for a rendered element is updated during the POST phase. Changing the ref
will queue up any effects that are dependent on that ref
. They'll be added to either the PRE or POST queue depending on their flush
setting. Either way, they won't be processed until the next loop around the render cycle.
So even if an effect uses flush: 'pre'
it still shouldn't get out of sync with the ref
. In practice using 'pre'
will cause it to be synced up sooner, so it could be argued that it's a better choice in some cases.
For an element ref
, the most obvious difference between 'pre'
and 'post'
is when the effect is run for the first time. The initial run for 'pre'
is before the component is mounted, so the ref
won't have been updated yet. But it will still be added to the queue when the ref
is updated, so it should still be called with the correct element just after mounting.
So why doesn't it work in vuejs/core#3030?
That's a bug. When it reaches the end of the render cycle it only checks whether the RENDER and POST queues are empty. It doesn't currently check the PRE queue. I reported this bug in vuejs/core#2730. There's a PR associated with that issue that should hopefully fix it. As you might notice, someone on that issue tried to argue that this is 'by design'. I believe that to be the same misunderstanding that I've outlined above. It fails to take account of the looping nature of the render cycle. With the current bug, you end up with unprocessed items leftover in the PRE queue when the current tick completes.
To further illustrate my point, consider the following example:
https://jsfiddle.net/skirtle/e3rnbqpg/
The code is very similar to the example in vuejs/core#3030. The only significant change is that I've added a flush: 'post'
watcher, in addition to the original flush: 'pre'
watcher. Notice the logging. The flush: 'pre'
watcher now works perfectly. The reason this 'works' is because the flush: 'post'
watcher triggers another loop around the render cycle, which then processes the PRE queue. This should happen anyway but due to the bug I mentioned earlier it doesn't happen unless there's something in one of the other two queues.
I'd like to make a final quick note about some of the ways that my description above is a simplication:
- If there are multiple components involved then it gets a little bit more complicated. A single iteration of the render cycle can actually end up running the PRE and RENDER queues multiple times, allowing it to handle
'pre'
watchers on props. Such watchers need to run after the parent is rendered but before the child renders. This process is further complicated because parent/child rendering circumvents the usual queuing mechanism for rendering. - The initial mounting that occurs when calling
app.mount(...)
behaves somewhat differently because it doesn't go through the scheduler queue in the normal way. I don't want to go into detail here but that's a can-of-worms all of its own. Here's an example, based on the earlier example, but where everything happens during the initial application mounting https://jsfiddle.net/skirtle/23jaeq4s/1/. Notice that the watcher works fine, despite not usingflush: 'post'
. The first time it's called theref
value isundefined
but that's to be expected. The key thing is that is does get called again when theref
is updated.
Co-authored-by: skirtle <[email protected]>
Co-authored-by: skirtle <[email protected]>
Co-authored-by: skirtle <[email protected]>
Co-authored-by: skirtle <[email protected]>
Co-authored-by: skirtle <[email protected]>
Co-authored-by: skirtle <[email protected]>
@skirtles-code Thanks for the detailed writeup. I'll try and look into this. But my first intuition is that not flushing pre-effects that were added to the scheduler queue during an update is done on purpose and with a reason. After all, In the case of a template ref, that's not an issue, as the effect make sense to run in the But at the moment I can't rule out (but don't have an example come to mind immediately either) that other As I said, this is just an intuition more or less, might be totally wrong ^^ |
This comment is a response to the previous comment from @LinusBorg but it is probably not directly relevant to the documentation. I'll post any thoughts I have about the PR itself separately. I don't think this was intentional but I can't be sure. During the RC phase I reported a number of bugs with pre watchers. At that point, pre effects didn't have their own queue, they were bundled into the main scheduler queue. Evan split the queue in this commit, as part of trying to fix some of the bugs I reported: vuejs/core@3692f27#diff-6142a09a188ff9ccb70f2ac08cb772e3ad9798386f3af4c95dd1567d9ab376ceR203 The GitHub diff isn't super helpful. The most relevant changes are in If it had been intentional then it would have been trivial to add in a warning to avoid the potential confusion. The two JSFiddles I included in my previous comment show that a pre effect can run after the post stage in some cases. If it is a deliberate design decision to stop pre effects running then these various edge cases also need to be addressed because they aren't consistent with that design. I think a key point that is worth re-iterating is that using In my opinion, the correct behaviour is for all effects to be processed within the same tick. The intuitive notion of three clearly distinct stages, pre/render/post, doesn't really reflect reality. Edge cases around watching props already lead to pre effects being flushed during the render phase, so those phases actually overlap (the code for that is in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good to merge.
I still think we're documenting around a bug, but given the current behaviour it does make sense to have this in the documentation. People keep hitting this problem.
Description of Problem
flush: 'pre'
)flush: 'post'
has to be used in order to get a correct element reference at all times.Proposed Solution
Additional Information
ref
of directive in v-else branch core#3030