fix(watch): should not fire pre watcher on child component unmount #7181
Conversation
| @@ -360,7 +360,10 @@ function doWatch( | |||
| } else { | |||
| // default: 'pre' | |||
| job.pre = true | |||
| if (instance) job.id = instance.uid | |||
| if (instance) { | |||
There was a problem hiding this comment.
this change is unnecessary.
we just check cb.id !== instance.uid in flushPreFlushCbs will be ok.
There was a problem hiding this comment.
Thanks. Updated.
| } | ||
|
|
||
| const Parent = { | ||
| porps: ['a'], |
There was a problem hiding this comment.
porps seems to be a typo. This line currently isn't doing anything.
There was a problem hiding this comment.
It's necessary to reproduce the issue.
There was a problem hiding this comment.
Are you sure? The spelling is wrong. It says porps, not props. There's no such option as porps.
There was a problem hiding this comment.
Oh, I see. Thanks.
| if (instance && cb.id !== instance.uid) { | ||
| continue | ||
| } |
There was a problem hiding this comment.
Shouldn't this come before the checkRecursiveUpdates check? We shouldn't be incrementing the count in seen unless we're actually running the job.
Maybe something like:
if (cb && cb.pre && (!instance || cb.id === instance.id)) {There was a problem hiding this comment.
Thanks. Updated.
I prefer to this condition instance && cb.id !== instance.uid that more clearer.
| // #7030 | ||
| it('should not fire on child component unmount w/ flush: pre', async () => { | ||
| const visible = ref(true) | ||
| const cb = jest.fn() |
There was a problem hiding this comment.
jest.fn() has been replaced with vi.fn() on the latest main branch.
There was a problem hiding this comment.
Thanks for reminding.
|
Is there an easy way to test if this PR works? |
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
|
Maybe ecosystem-ci should be triggered after rebase. |
Size ReportBundles
Usages
|
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
This fixes drafts not clearing after posting a reply. Vue 3.3.11 changed watchers to stop firing after component unmount. After posting a reply, the post form is removed, now causing the queued event to be discarded. Synchronous flush causes the handler to be called immediately when changes happen, solving the problem. The performance impact of this change seems non-existent. Even before, typing would generate an event for each keystroke. Pasting is atomic. See: vuejs/core#7181 See: vuejs/docs@80e2128 Fixes: a7dea2f Fixes: #413
Fixes #7030