From 2e68b3ce70aedd6a9a48cf39e390e3ae937b9b5a Mon Sep 17 00:00:00 2001 From: Haoqun Jiang Date: Thu, 20 Dec 2018 02:29:00 +0800 Subject: [PATCH] fix: always use microtasks for nextTick (#8450) fix #7109, #7546, #7707, #7834, #8109 reopen #6566 --- src/core/util/next-tick.js | 117 ++++++++---------- .../web/runtime/modules/dom-props.js | 11 ++ src/platforms/web/runtime/modules/events.js | 3 +- test/e2e/specs/async-edge-cases.js | 24 ++-- 4 files changed, 76 insertions(+), 79 deletions(-) diff --git a/src/core/util/next-tick.js b/src/core/util/next-tick.js index cc066ed86f6..b63e55c9715 100644 --- a/src/core/util/next-tick.js +++ b/src/core/util/next-tick.js @@ -1,9 +1,9 @@ /* @flow */ -/* globals MessageChannel */ +/* globals MutationObserver */ import { noop } from 'shared/util' import { handleError } from './error' -import { isIOS, isNative } from './env' +import { isIE, isIOS, isNative } from './env' const callbacks = [] let pending = false @@ -17,76 +17,67 @@ function flushCallbacks () { } } -// Here we have async deferring wrappers using both microtasks and (macro) tasks. -// In < 2.4 we used microtasks everywhere, but there are some scenarios where -// microtasks have too high a priority and fire in between supposedly -// sequential events (e.g. #4521, #6690) or even between bubbling of the same -// event (#6566). However, using (macro) tasks everywhere also has subtle problems -// when state is changed right before repaint (e.g. #6813, out-in transitions). -// Here we use microtask by default, but expose a way to force (macro) task when -// needed (e.g. in event handlers attached by v-on). -let microTimerFunc -let macroTimerFunc -let useMacroTask = false +// Here we have async deferring wrappers using microtasks. +// In 2.5 we used (macro) tasks (in combination with microtasks). +// However, it has subtle problems when state is changed right before repaint +// (e.g. #6813, out-in transitions). +// Also, using (macro) tasks in event handler would cause some weird behaviors +// that cannot be circumvented (e.g. #7109, #7153, #7546, #7834, #8109). +// So we now use microtasks everywhere, again. +// A major drawback of this tradeoff is that there are some scenarios +// where microtasks have too high a priority and fire in between supposedly +// sequential events (e.g. #4521, #6690, which have workarounds) +// or even between bubbling of the same event (#6566). +let timerFunc -// Determine (macro) task defer implementation. -// Technically setImmediate should be the ideal choice, but it's only available -// in IE. The only polyfill that consistently queues the callback after all DOM -// events triggered in the same loop is by using MessageChannel. -/* istanbul ignore if */ -if (typeof setImmediate !== 'undefined' && isNative(setImmediate)) { - macroTimerFunc = () => { - setImmediate(flushCallbacks) - } -} else if (typeof MessageChannel !== 'undefined' && ( - isNative(MessageChannel) || - // PhantomJS - MessageChannel.toString() === '[object MessageChannelConstructor]' -)) { - const channel = new MessageChannel() - const port = channel.port2 - channel.port1.onmessage = flushCallbacks - macroTimerFunc = () => { - port.postMessage(1) - } -} else { - /* istanbul ignore next */ - macroTimerFunc = () => { - setTimeout(flushCallbacks, 0) - } -} - -// Determine microtask defer implementation. +// The nextTick behavior leverages the microtask queue, which can be accessed +// via either native Promise.then or MutationObserver. +// MutationObserver has wider support, however it is seriously bugged in +// UIWebView in iOS >= 9.3.3 when triggered in touch event handlers. It +// completely stops working after triggering a few times... so, if native +// Promise is available, we will use it: /* istanbul ignore next, $flow-disable-line */ if (typeof Promise !== 'undefined' && isNative(Promise)) { const p = Promise.resolve() - microTimerFunc = () => { + timerFunc = () => { p.then(flushCallbacks) - // in problematic UIWebViews, Promise.then doesn't completely break, but + // In problematic UIWebViews, Promise.then doesn't completely break, but // it can get stuck in a weird state where callbacks are pushed into the // microtask queue but the queue isn't being flushed, until the browser // needs to do some other work, e.g. handle a timer. Therefore we can // "force" the microtask queue to be flushed by adding an empty timer. if (isIOS) setTimeout(noop) } -} else { - // fallback to macro - microTimerFunc = macroTimerFunc -} - -/** - * Wrap a function so that if any code inside triggers state change, - * the changes are queued using a (macro) task instead of a microtask. - */ -export function withMacroTask (fn: Function): Function { - return fn._withTask || (fn._withTask = function () { - useMacroTask = true - try { - return fn.apply(null, arguments) - } finally { - useMacroTask = false - } +} else if (!isIE && typeof MutationObserver !== 'undefined' && ( + isNative(MutationObserver) || + // PhantomJS and iOS 7.x + MutationObserver.toString() === '[object MutationObserverConstructor]' +)) { + // Use MutationObserver where native Promise is not available, + // e.g. PhantomJS, iOS7, Android 4.4 + // (#6466 MutationObserver is unreliable in IE11) + let counter = 1 + const observer = new MutationObserver(flushCallbacks) + const textNode = document.createTextNode(String(counter)) + observer.observe(textNode, { + characterData: true }) + timerFunc = () => { + counter = (counter + 1) % 2 + textNode.data = String(counter) + } +} else if (typeof setImmediate !== 'undefined' && isNative(setImmediate)) { + // Fallback to setImmediate. + // Techinically it leverages the (macro) task queue, + // but it is still a better choice than setTimeout. + timerFunc = () => { + setImmediate(flushCallbacks) + } +} else { + // Fallback to setTimeout. + timerFunc = () => { + setTimeout(flushCallbacks, 0) + } } export function nextTick (cb?: Function, ctx?: Object) { @@ -104,11 +95,7 @@ export function nextTick (cb?: Function, ctx?: Object) { }) if (!pending) { pending = true - if (useMacroTask) { - macroTimerFunc() - } else { - microTimerFunc() - } + timerFunc() } // $flow-disable-line if (!cb && typeof Promise !== 'undefined') { diff --git a/src/platforms/web/runtime/modules/dom-props.js b/src/platforms/web/runtime/modules/dom-props.js index f3888aed066..ceec663ac33 100644 --- a/src/platforms/web/runtime/modules/dom-props.js +++ b/src/platforms/web/runtime/modules/dom-props.js @@ -35,6 +35,17 @@ function updateDOMProps (oldVnode: VNodeWithData, vnode: VNodeWithData) { } } + // #4521: if a click event triggers update before the change event is + // dispatched on a checkbox/radio input, the input's checked state will + // be reset and fail to trigger another update. + // The root cause here is that browsers may fire microtasks in between click/change. + // In Chrome / Firefox, click event fires before change, thus having this problem. + // In Safari / Edge, the order is opposite. + // Note: in Edge, if you click too fast, only the click event would fire twice. + if (key === 'checked' && !isNotInFocusAndDirty(elm, cur)) { + continue + } + if (key === 'value') { // store value as _value as well since // non-string values will be stringified diff --git a/src/platforms/web/runtime/modules/events.js b/src/platforms/web/runtime/modules/events.js index e89a38d82c9..1f44a106412 100644 --- a/src/platforms/web/runtime/modules/events.js +++ b/src/platforms/web/runtime/modules/events.js @@ -2,7 +2,7 @@ import { isDef, isUndef } from 'shared/util' import { updateListeners } from 'core/vdom/helpers/index' -import { withMacroTask, isIE, supportsPassive } from 'core/util/index' +import { isIE, supportsPassive } from 'core/util/index' import { RANGE_TOKEN, CHECKBOX_RADIO_TOKEN } from 'web/compiler/directives/model' // normalize v-model event tokens that can only be determined at runtime. @@ -44,7 +44,6 @@ function add ( capture: boolean, passive: boolean ) { - handler = withMacroTask(handler) target.addEventListener( event, handler, diff --git a/test/e2e/specs/async-edge-cases.js b/test/e2e/specs/async-edge-cases.js index 2873409400b..077387293c6 100644 --- a/test/e2e/specs/async-edge-cases.js +++ b/test/e2e/specs/async-edge-cases.js @@ -14,20 +14,20 @@ module.exports = { .assert.containsText('#case-1', '3') .assert.checked('#case-1 input', false) - // #6566 - .assert.containsText('#case-2 button', 'Expand is True') - .assert.containsText('.count-a', 'countA: 0') - .assert.containsText('.count-b', 'countB: 0') + // // #6566 + // .assert.containsText('#case-2 button', 'Expand is True') + // .assert.containsText('.count-a', 'countA: 0') + // .assert.containsText('.count-b', 'countB: 0') - .click('#case-2 button') - .assert.containsText('#case-2 button', 'Expand is False') - .assert.containsText('.count-a', 'countA: 1') - .assert.containsText('.count-b', 'countB: 0') + // .click('#case-2 button') + // .assert.containsText('#case-2 button', 'Expand is False') + // .assert.containsText('.count-a', 'countA: 1') + // .assert.containsText('.count-b', 'countB: 0') - .click('#case-2 button') - .assert.containsText('#case-2 button', 'Expand is True') - .assert.containsText('.count-a', 'countA: 1') - .assert.containsText('.count-b', 'countB: 1') + // .click('#case-2 button') + // .assert.containsText('#case-2 button', 'Expand is True') + // .assert.containsText('.count-a', 'countA: 1') + // .assert.containsText('.count-b', 'countB: 1') .end() }