Skip to content

Commit

Permalink
Merge pull request #958 from emberjs/avoid-rsvp-promises-internally
Browse files Browse the repository at this point in the history
  • Loading branch information
rwjblue authored Dec 4, 2020
2 parents 400c0b2 + 8c33a27 commit 6e9988f
Show file tree
Hide file tree
Showing 33 changed files with 2,765 additions and 231 deletions.
12 changes: 9 additions & 3 deletions addon-test-support/@ember/test-helpers/-internal/helper-hooks.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { _Promise as Promise } from '../-utils';
import { Promise } from '../-utils';

type Hook = (...args: any[]) => void | Promise<void>;
type HookLabel = 'start' | 'end' | string;
Expand All @@ -14,7 +14,7 @@ const registeredHooks = new Map<string, Set<Hook>>();
* @param {string} label A label to help identify the hook.
* @returns {string} The compound key for the helper.
*/
function getHelperKey(helperName: string, label: string) {
function getHelperKey(helperName: string, label: string): string {
return `${helperName}:${label}`;
}

Expand Down Expand Up @@ -59,7 +59,13 @@ export function registerHook(helperName: string, label: HookLabel, hook: Hook):
*/
export function runHooks(helperName: string, label: HookLabel, ...args: any[]): Promise<void> {
let hooks = registeredHooks.get(getHelperKey(helperName, label)) || new Set<Hook>();
let promises = [...hooks].map(hook => hook(...args));
let promises: Array<void | Promise<void>> = [];

hooks.forEach(hook => {
let hookResult = hook(...args);

promises.push(hookResult);
});

return Promise.all(promises).then(() => {});
}
91 changes: 8 additions & 83 deletions addon-test-support/@ember/test-helpers/-utils.ts
Original file line number Diff line number Diff line change
@@ -1,96 +1,21 @@
/* globals Promise */

import RSVP from 'rsvp';
import hasEmberVersion from './has-ember-version';

export class _Promise<T> extends RSVP.Promise<T> {}
const HAS_PROMISE =
typeof Promise === 'function' &&
// @ts-ignore this is checking if someone has explicitly done `window.Promise = window.Promise || Ember.RSVP.Promise
Promise !== RSVP.Promise;

const ORIGINAL_RSVP_ASYNC: Function = RSVP.configure('async');
import { Promise as PromisePolyfill } from 'es6-promise';

/*
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`:
const _Promise: typeof Promise = HAS_PROMISE ? Promise : (PromisePolyfill as typeof Promise);

```js
console.log('first');
Ember.run(() => Promise.resolve().then(() => console.log('second')));
console.log('third');
```
export { _Promise as Promise };

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 configured `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: any, promise: any) => {
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: () => void) => Promise.resolve().then(cb);
export const nextTick = HAS_PROMISE ? (cb: () => void) => Promise.resolve().then(cb) : RSVP.asap;
export const futureTick = setTimeout;

/**
@private
@returns {Promise<void>} Promise which can not be forced to be ran synchronously
*/
export function nextTickPromise(): RSVP.Promise<void> {
// 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);
});
}
}

/**
Retrieves an array of destroyables from the specified property on the object
provided, iterates that array invoking each function, then deleting the
Expand Down
3 changes: 2 additions & 1 deletion addon-test-support/@ember/test-helpers/build-owner.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Application from '@ember/application';
import Resolver from '@ember/application/resolver';

import { Promise } from 'rsvp';
import { Promise } from './-utils';

import legacyBuildRegistry from './-internal/build-registry';
import ContainerProxyMixin from '@ember/engine/-private/container-proxy-mixin';
Expand Down Expand Up @@ -52,5 +52,6 @@ export default function buildOwner(
}

let { owner } = legacyBuildRegistry(resolver) as { owner: Owner };

return Promise.resolve(owner);
}
4 changes: 2 additions & 2 deletions addon-test-support/@ember/test-helpers/dom/blur.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import getElement from './-get-element';
import fireEvent from './fire-event';
import settled from '../settled';
import { nextTickPromise } from '../-utils';
import { Promise } from '../-utils';
import Target from './-target';
import { log } from '@ember/test-helpers/dom/-logging';
import isFocusable from './-is-focusable';
Expand Down Expand Up @@ -61,7 +61,7 @@ export function __blur__(element: HTMLElement | Element | Document | SVGElement)
blur('input');
*/
export default function blur(target: Target = document.activeElement!): Promise<void> {
return nextTickPromise()
return Promise.resolve()
.then(() => runHooks('blur', 'start', target))
.then(() => {
let element = getElement(target);
Expand Down
4 changes: 2 additions & 2 deletions addon-test-support/@ember/test-helpers/dom/click.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import fireEvent from './fire-event';
import { __focus__ } from './focus';
import settled from '../settled';
import isFocusable from './-is-focusable';
import { nextTickPromise } from '../-utils';
import { Promise } from '../-utils';
import isFormControl from './-is-form-control';
import Target from './-target';
import { log } from '@ember/test-helpers/dom/-logging';
Expand Down Expand Up @@ -90,7 +90,7 @@ export function __click__(element: Element | Document | Window, options: MouseEv
export default function click(target: Target, _options: MouseEventInit = {}): Promise<void> {
let options = assign({}, DEFAULT_CLICK_OPTIONS, _options);

return nextTickPromise()
return Promise.resolve()
.then(() => runHooks('click', 'start', target, _options))
.then(() => {
if (!target) {
Expand Down
4 changes: 2 additions & 2 deletions addon-test-support/@ember/test-helpers/dom/double-click.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import fireEvent from './fire-event';
import { __focus__ } from './focus';
import settled from '../settled';
import isFocusable from './-is-focusable';
import { nextTickPromise } from '../-utils';
import { Promise } from '../-utils';
import { DEFAULT_CLICK_OPTIONS } from './click';
import Target from './-target';
import { log } from '@ember/test-helpers/dom/-logging';
Expand Down Expand Up @@ -94,7 +94,7 @@ export function __doubleClick__(
export default function doubleClick(target: Target, _options: MouseEventInit = {}): Promise<void> {
let options = assign({}, DEFAULT_CLICK_OPTIONS, _options);

return nextTickPromise()
return Promise.resolve()
.then(() => runHooks('doubleClick', 'start', target, _options))
.then(() => {
if (!target) {
Expand Down
4 changes: 2 additions & 2 deletions addon-test-support/@ember/test-helpers/dom/fill-in.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import guardForMaxlength from './-guard-for-maxlength';
import { __focus__ } from './focus';
import settled from '../settled';
import fireEvent from './fire-event';
import { nextTickPromise } from '../-utils';
import { Promise } from '../-utils';
import Target, { isContentEditable } from './-target';
import { log } from '@ember/test-helpers/dom/-logging';
import { runHooks, registerHook } from '../-internal/helper-hooks';
Expand All @@ -31,7 +31,7 @@ registerHook('fillIn', 'start', (target: Target, text: string) => {
fillIn('input', 'hello world');
*/
export default function fillIn(target: Target, text: string): Promise<void> {
return nextTickPromise()
return Promise.resolve()
.then(() => runHooks('fillIn', 'start', target, text))
.then(() => {
if (!target) {
Expand Down
4 changes: 2 additions & 2 deletions addon-test-support/@ember/test-helpers/dom/focus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import getElement from './-get-element';
import fireEvent from './fire-event';
import settled from '../settled';
import isFocusable from './-is-focusable';
import { nextTickPromise } from '../-utils';
import { Promise } from '../-utils';
import Target from './-target';
import { log } from '@ember/test-helpers/dom/-logging';
import { runHooks, registerHook } from '../-internal/helper-hooks';
Expand Down Expand Up @@ -73,7 +73,7 @@ export function __focus__(element: HTMLElement | Element | Document | SVGElement
focus('input');
*/
export default function focus(target: Target): Promise<void> {
return nextTickPromise()
return Promise.resolve()
.then(() => runHooks('focus', 'start', target))
.then(() => {
if (!target) {
Expand Down
4 changes: 2 additions & 2 deletions addon-test-support/@ember/test-helpers/dom/scroll-to.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import getElement from './-get-element';
import fireEvent from './fire-event';
import settled from '../settled';
import { nextTickPromise } from '../-utils';
import { Promise } from '../-utils';
import { isElement } from './-target';
import { runHooks } from '../-internal/helper-hooks';

Expand All @@ -26,7 +26,7 @@ export default function scrollTo(
x: number,
y: number
): Promise<void> {
return nextTickPromise()
return Promise.resolve()
.then(() => runHooks('scrollTo', 'start', target))
.then(() => {
if (!target) {
Expand Down
4 changes: 2 additions & 2 deletions addon-test-support/@ember/test-helpers/dom/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import isSelectElement from './-is-select-element';
import { __focus__ } from './focus';
import settled from '../settled';
import fireEvent from './fire-event';
import { nextTickPromise } from '../-utils';
import { Promise } from '../-utils';
import Target from './-target';
import { runHooks } from '../-internal/helper-hooks';

Expand Down Expand Up @@ -35,7 +35,7 @@ export default function select(
options: string | string[],
keepPreviouslySelected = false
): Promise<void> {
return nextTickPromise()
return Promise.resolve()
.then(() => runHooks('select', 'start', target, options, keepPreviouslySelected))
.then(() => {
if (!target) {
Expand Down
4 changes: 2 additions & 2 deletions addon-test-support/@ember/test-helpers/dom/tap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import getElement from './-get-element';
import fireEvent from './fire-event';
import { __click__ } from './click';
import settled from '../settled';
import { nextTickPromise } from '../-utils';
import { Promise } from '../-utils';
import Target from './-target';
import { log } from '@ember/test-helpers/dom/-logging';
import isFormControl from './-is-form-control';
Expand Down Expand Up @@ -55,7 +55,7 @@ registerHook('tap', 'start', (target: Target) => {
tap('button');
*/
export default function tap(target: Target, options: object = {}): Promise<void> {
return nextTickPromise()
return Promise.resolve()
.then(() => {
return runHooks('tap', 'start', target, options);
})
Expand Down
4 changes: 2 additions & 2 deletions addon-test-support/@ember/test-helpers/dom/trigger-event.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { getWindowOrElement } from './-get-window-or-element';
import fireEvent from './fire-event';
import settled from '../settled';
import { nextTickPromise } from '../-utils';
import { Promise } from '../-utils';
import Target from './-target';
import { log } from '@ember/test-helpers/dom/-logging';
import isFormControl from './-is-form-control';
Expand Down Expand Up @@ -59,7 +59,7 @@ export default function triggerEvent(
eventType: string,
options?: object
): Promise<void> {
return nextTickPromise()
return Promise.resolve()
.then(() => {
return runHooks('triggerEvent', 'start', target, eventType, options);
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import getElement from './-get-element';
import fireEvent from './fire-event';
import settled from '../settled';
import { KEYBOARD_EVENT_TYPES, KeyboardEventType, isKeyboardEventType } from './fire-event';
import { nextTickPromise, isNumeric } from '../-utils';
import { Promise, isNumeric } from '../-utils';
import Target from './-target';
import { log } from '@ember/test-helpers/dom/-logging';
import isFormControl from './-is-form-control';
Expand Down Expand Up @@ -194,7 +194,7 @@ export default function triggerKeyEvent(
key: number | string,
modifiers: KeyModifiers = DEFAULT_MODIFIERS
): Promise<void> {
return nextTickPromise()
return Promise.resolve()
.then(() => {
return runHooks('triggerKeyEvent', 'start', target, eventType, key);
})
Expand Down
7 changes: 3 additions & 4 deletions addon-test-support/@ember/test-helpers/dom/type-in.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { nextTickPromise } from '../-utils';
import { Promise } from '../-utils';
import settled from '../settled';
import getElement from './-get-element';
import isFormControl, { FormControl } from './-is-form-control';
import { __focus__ } from './focus';
import { Promise } from 'rsvp';
import fireEvent from './fire-event';
import guardForMaxlength from './-guard-for-maxlength';
import Target, { isContentEditable, isDocument, HTMLElementContentEditable } from './-target';
Expand Down Expand Up @@ -44,7 +43,7 @@ registerHook('typeIn', 'start', (target: Target, text: string) => {
* typeIn('input', 'hello world');
*/
export default function typeIn(target: Target, text: string, options: Options = {}): Promise<void> {
return nextTickPromise()
return Promise.resolve()
.then(() => {
return runHooks('typeIn', 'start', target, text, options);
})
Expand Down Expand Up @@ -108,7 +107,7 @@ function keyEntry(
let characterKey = character.toUpperCase();

return function () {
return nextTickPromise()
return Promise.resolve()
.then(() => __triggerKeyEvent__(element, 'keydown', characterKey, options))
.then(() => __triggerKeyEvent__(element, 'keypress', characterKey, options))
.then(() => {
Expand Down
4 changes: 2 additions & 2 deletions addon-test-support/@ember/test-helpers/dom/wait-for.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import waitUntil from '../wait-until';
import getElement from './-get-element';
import getElements from './-get-elements';
import toArray from './-to-array';
import { nextTickPromise } from '../-utils';
import { Promise } from '../-utils';

export interface Options {
timeout?: number;
Expand Down Expand Up @@ -31,7 +31,7 @@ export default function waitFor(
selector: string,
options: Options = {}
): Promise<Element | Element[]> {
return nextTickPromise().then(() => {
return Promise.resolve().then(() => {
if (!selector) {
throw new Error('Must pass a selector to `waitFor`.');
}
Expand Down
Loading

0 comments on commit 6e9988f

Please sign in to comment.