From 0da35f418a603eb060b1c89ae2b0b778fe2cef0f Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 2 Nov 2018 19:40:12 -0400 Subject: [PATCH] Ensure `await settled()` avoids creating an autorun. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Long ago in a galaxy far far away, Ember forced RSVP.Promise to "resolve" on the Ember.run loop. At the time, this was meant to help ease pain with folks receiving the dreaded "auto-run" assertion during their tests, and to help ensure that promise resolution was coelesced to avoid "thrashing" of the DOM. Unfortunately, the result of this configuration is that code like the following behaves differently if using native `Promise` vs `RSVP.Promise`: ```js console.log('first'); Ember.run(() => Promise.resolve().then(() => console.log('second'))); console.log('third'); ``` When `Promise` is the native promise that will log `'first', 'third', 'second'`, but when `Promise` is an `RSVP.Promise` that will log `'first', 'second', 'third'`. The fact that `RSVP.Promise`s can be **forced** to flush synchronously is very scary! Now, lets talk about why we are configuring `RSVP`'s `async` below... --- The following _should_ always be guaranteed: ```js await settled(); isSettled() === true ``` Unfortunately, without the custom `RSVP` `async` configuration we cannot ensure that `isSettled()` will be truthy. This is due to the fact that Ember has configurate `RSVP` to resolve all promises in the run loop. What that means practically is this: 1. all checks within `waitUntil` (used by `settled()` internally) are completed and we are "settled" 2. `waitUntil` resolves the promise that it returned (to signify that the world is "settled") 3. resolving the promise (since it is an `RSVP.Promise` and Ember has configured RSVP.Promise) creates a new Ember.run loop in order to resolve 4. the presence of that new run loop means that we are no longer "settled" 5. `isSettled()` returns false 😭😭😭😭😭😭😭😭😭 This custom `RSVP.configure('async`, ...)` below provides a way to prevent the promises that are returned from `settled` from causing this "loop" and instead "just use normal Promise semantics". 😩😫🙀 --- .../@ember/test-helpers/-utils.ts | 84 ++++++++++++++++++- .../@ember/test-helpers/wait-until.ts | 3 +- 2 files changed, 81 insertions(+), 6 deletions(-) diff --git a/addon-test-support/@ember/test-helpers/-utils.ts b/addon-test-support/@ember/test-helpers/-utils.ts index eaebe2b8c..c9edc438a 100644 --- a/addon-test-support/@ember/test-helpers/-utils.ts +++ b/addon-test-support/@ember/test-helpers/-utils.ts @@ -1,5 +1,74 @@ /* globals Promise */ -import { Promise as RSVPPromise } from 'rsvp'; + +import RSVP from 'rsvp'; +import hasEmberVersion from './has-ember-version'; + +export class _Promise extends RSVP.Promise {} + +const ORIGINAL_RSVP_ASYNC: Function = RSVP.configure('async'); + +/* + Long ago in a galaxy far far away, Ember forced RSVP.Promise to "resolve" on the Ember.run loop. + At the time, this was meant to help ease pain with folks receiving the dreaded "auto-run" assertion + during their tests, and to help ensure that promise resolution was coelesced to avoid "thrashing" + of the DOM. Unfortunately, the result of this configuration is that code like the following behaves + differently if using native `Promise` vs `RSVP.Promise`: + + ```js + console.log('first'); + Ember.run(() => Promise.resolve().then(() => console.log('second'))); + console.log('third'); + ``` + + When `Promise` is the native promise that will log `'first', 'third', 'second'`, but when `Promise` + is an `RSVP.Promise` that will log `'first', 'second', 'third'`. The fact that `RSVP.Promise`s can + be **forced** to flush synchronously is very scary! + + Now, lets talk about why we are configuring `RSVP`'s `async` below... + + --- + + The following _should_ always be guaranteed: + + ```js + await settled(); + + isSettled() === true + ``` + + Unfortunately, without the custom `RSVP` `async` configuration we cannot ensure that `isSettled()` will + be truthy. This is due to the fact that Ember has configurate `RSVP` to resolve all promises in the run + loop. What that means practically is this: + + 1. all checks within `waitUntil` (used by `settled()` internally) are completed and we are "settled" + 2. `waitUntil` resolves the promise that it returned (to signify that the world is "settled") + 3. resolving the promise (since it is an `RSVP.Promise` and Ember has configured RSVP.Promise) creates + a new Ember.run loop in order to resolve + 4. the presence of that new run loop means that we are no longer "settled" + 5. `isSettled()` returns false 😭😭😭😭😭😭😭😭😭 + + This custom `RSVP.configure('async`, ...)` below provides a way to prevent the promises that are returned + from `settled` from causing this "loop" and instead "just use normal Promise semantics". + + 😩😫🙀 +*/ +RSVP.configure('async', (callback, promise) => { + if (promise instanceof _Promise) { + // @ts-ignore - avoid erroring about useless `Promise !== RSVP.Promise` comparison + // (this handles when folks have polyfilled via Promise = Ember.RSVP.Promise) + if (typeof Promise !== 'undefined' && Promise !== RSVP.Promise) { + // use real native promise semantics whenever possible + Promise.resolve().then(() => callback(promise)); + } else { + // fallback to using RSVP's natural `asap` (**not** the fake + // one configured by Ember...) + RSVP.asap(callback, promise); + } + } else { + // fall back to the normal Ember behavior + ORIGINAL_RSVP_ASYNC(callback, promise); + } +}); export const nextTick: Function = typeof Promise === 'undefined' ? setTimeout : cb => Promise.resolve().then(cb); @@ -10,9 +79,16 @@ export const futureTick = setTimeout; @returns {Promise} Promise which can not be forced to be ran synchronously */ export function nextTickPromise() { - return new RSVPPromise(resolve => { - nextTick(resolve); - }); + // Ember 3.4 removed the auto-run assertion, in 3.4+ we can (and should) avoid the "psuedo promisey" run loop configuration + // for our `nextTickPromise` implementation. This allows us to have real microtask based next tick timing... + if (hasEmberVersion(3,4)) { + return _Promise.resolve(); + } else { + // on older Ember's fallback to RSVP.Promise + a setTimeout + return new RSVP.Promise(resolve => { + nextTick(resolve); + }); + } } /** diff --git a/addon-test-support/@ember/test-helpers/wait-until.ts b/addon-test-support/@ember/test-helpers/wait-until.ts index 1b430300d..f5af37d13 100644 --- a/addon-test-support/@ember/test-helpers/wait-until.ts +++ b/addon-test-support/@ember/test-helpers/wait-until.ts @@ -1,5 +1,4 @@ -import { Promise } from 'rsvp'; -import { futureTick } from './-utils'; +import { futureTick, _Promise as Promise } from './-utils'; const TIMEOUTS = [0, 1, 2, 5, 7]; const MAX_TIMEOUT = 10;