Skip to content

Commit

Permalink
Upgrade to Ember v4 types
Browse files Browse the repository at this point in the history
Since this library was not generally using deprecated APIs, this simply
allows us to get the latest and greatest type definitions. The one
deprecated API that *is* in use is direct access to the `run`
namespace. However, that is already covered via a `hasEmberVersion`
check.

In several cases, introduce additional safety handling where TypeScript
can now identify failure modes (both because of improvements to the
compiler and because our tsconfig is now stricter). In others,
introduce hard errors for cases where TypeScript points out the need
for *some* variety of validation, but the failure scenario is one where
there is nothing actionable whatsoever.

Along the way, fix a number of lingering type errors, some of which
were pedantic but a number of which represented actual failure
conditions.
  • Loading branch information
chriskrycho committed Jul 21, 2022
1 parent db9a6d3 commit 30e9c1f
Show file tree
Hide file tree
Showing 21 changed files with 210 additions and 534 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type Resolver from '@ember/application/resolver';
import type Resolver from 'ember-resolver';
import ApplicationInstance from '@ember/application/instance';
import Application from '@ember/application';
import EmberObject from '@ember/object';
Expand Down Expand Up @@ -31,9 +31,11 @@ function exposeRegistryMethodsWithoutDeprecations(container: any) {
for (let i = 0, l = methods.length; i < l; i++) {
let method = methods[i];

if (method in container) {
if (method && method in container) {
container[method] = function (...args: unknown[]) {
return container._registry[method](...args);
// SAFETY: `method` is defined because we *just* checked that it is in
// the conditional wrapping this.
return container._registry[method!](...args);
};
}
}
Expand All @@ -57,10 +59,10 @@ const Owner = EmberObject.extend(RegistryProxyMixin, ContainerProxyMixin, {
* @see {@link https://github.com/emberjs/ember.js/blob/v4.5.0-alpha.5/packages/%40ember/engine/instance.ts#L152-L167}
*/
unregister(fullName: string) {
this.__container__.reset(fullName);
this['__container__'].reset(fullName);

// We overwrote this method from RegistryProxyMixin.
this.__registry__.unregister(fullName);
this['__registry__'].unregister(fullName);
},
});

Expand Down
55 changes: 24 additions & 31 deletions addon-test-support/@ember/test-helpers/-internal/debug-info.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,17 @@
import { _backburner } from '@ember/runloop';
import {
_backburner,
DebugInfo as BackburnerDebugInfo,
QueueItem,
DeferredActionQueues,
} from '@ember/runloop';
} from '@ember/runloop/-private/backburner';
import { DebugInfoHelper, debugInfoHelpers } from './debug-info-helpers';
import {
getPendingWaiterState,
PendingWaiterState,
TestWaiterDebugInfo,
} from '@ember/test-waiters';
import { getPendingWaiterState, PendingWaiterState } from '@ember/test-waiters';

const PENDING_AJAX_REQUESTS = 'Pending AJAX requests';
const PENDING_TEST_WAITERS = 'Pending test waiters';
const SCHEDULED_ASYNC = 'Scheduled async';
const SCHEDULED_AUTORUN = 'Scheduled autorun';

type MaybeDebugInfo = BackburnerDebugInfo | null;
type WaiterDebugInfo = true | unknown[];

interface SettledState {
hasPendingTimers: boolean;
Expand All @@ -37,7 +31,7 @@ interface SummaryInfo {
pendingTimersCount: number;
hasPendingTimers: boolean;
pendingTimersStackTraces: (string | undefined)[];
pendingScheduledQueueItemCount: Number;
pendingScheduledQueueItemCount: number;
pendingScheduledQueueItemStackTraces: (string | undefined)[];
hasRunLoop: boolean;
isRenderPending: boolean;
Expand Down Expand Up @@ -112,31 +106,30 @@ export class TestDebugInfo implements DebugInfo {
this._summaryInfo.pendingScheduledQueueItemCount =
this._debugInfo.instanceStack
.filter((q) => q)
.reduce((total: Number, item) => {
Object.keys(item).forEach((queueName: string) => {
total += item[queueName].length;
.reduce((total, item) => {
Object.keys(item).forEach((queueName) => {
// SAFETY: this cast is *not* safe, but the underlying type is
// not currently able to be safer than this because it was
// built as a bag-of-queues *and* a structured item originally.
total += (item[queueName] as QueueItem[]).length;
});

return total;
}, 0);
this._summaryInfo.pendingScheduledQueueItemStackTraces =
this._debugInfo.instanceStack
.filter((q) => q)
.reduce(
(
stacks: string[],
deferredActionQueues: DeferredActionQueues
) => {
Object.keys(deferredActionQueues).forEach((queue) => {
deferredActionQueues[queue].forEach(
(queueItem: QueueItem) =>
queueItem.stack && stacks.push(queueItem.stack)
);
});
return stacks;
},
[]
);
.reduce((stacks, deferredActionQueues) => {
Object.keys(deferredActionQueues).forEach((queue) => {
// SAFETY: this cast is *not* safe, but the underlying type is
// not currently able to be safer than this because it was
// built as a bag-of-queues *and* a structured item originally.
(deferredActionQueues[queue] as QueueItem[]).forEach(
(queueItem) => queueItem.stack && stacks.push(queueItem.stack)
);
});
return stacks;
}, [] as string[]);
}

if (this._summaryInfo.hasPendingTestWaiters) {
Expand Down Expand Up @@ -165,12 +158,12 @@ export class TestDebugInfo implements DebugInfo {

Object.keys(summary.pendingTestWaiterInfo.waiters).forEach(
(waiterName) => {
let waiterDebugInfo: WaiterDebugInfo =
let waiterDebugInfo =
summary.pendingTestWaiterInfo.waiters[waiterName];

if (Array.isArray(waiterDebugInfo)) {
_console.group(waiterName);
waiterDebugInfo.forEach((debugInfo: TestWaiterDebugInfo) => {
waiterDebugInfo.forEach((debugInfo) => {
_console.log(
`${debugInfo.label ? debugInfo.label : 'stack'}: ${
debugInfo.stack
Expand Down Expand Up @@ -221,7 +214,7 @@ export class TestDebugInfo implements DebugInfo {
});
}

_formatCount(title: string, count: Number): string {
_formatCount(title: string, count: number): string {
return `${title}: ${count}`;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export interface DeprecationOptions {

export interface DeprecationFailure {
message: string;
options: DeprecationOptions;
options?: DeprecationOptions;
}

const DEPRECATIONS = new WeakMap<BaseContext, Array<DeprecationFailure>>();
Expand Down Expand Up @@ -92,7 +92,7 @@ if (typeof URLSearchParams !== 'undefined') {
// those deprecations will be squelched
if (disabledDeprecations) {
registerDeprecationHandler((message, options, next) => {
if (!disabledDeprecations.includes(options.id)) {
if (!options || !disabledDeprecations.includes(options.id)) {
next.apply(null, [message, options]);
}
});
Expand All @@ -102,7 +102,7 @@ if (typeof URLSearchParams !== 'undefined') {
// `some-other-thing` deprecation is triggered, this `debugger` will be hit`
if (debugDeprecations) {
registerDeprecationHandler((message, options, next) => {
if (debugDeprecations.includes(options.id)) {
if (options && debugDeprecations.includes(options.id)) {
debugger; // eslint-disable-line no-debugger
}

Expand Down
4 changes: 2 additions & 2 deletions addon-test-support/@ember/test-helpers/-internal/warnings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ if (typeof URLSearchParams !== 'undefined') {
// those warnings will be squelched
if (disabledWarnings) {
registerWarnHandler((message, options, next) => {
if (!disabledWarnings.includes(options.id)) {
if (!options || !disabledWarnings.includes(options.id)) {
next.apply(null, [message, options]);
}
});
Expand All @@ -99,7 +99,7 @@ if (typeof URLSearchParams !== 'undefined') {
// `some-other-thing` warning is triggered, this `debugger` will be hit`
if (debugWarnings) {
registerWarnHandler((message, options, next) => {
if (debugWarnings.includes(options.id)) {
if (options && debugWarnings.includes(options.id)) {
debugger; // eslint-disable-line no-debugger
}

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,5 +1,5 @@
import Application from '@ember/application';
import type Resolver from '@ember/application/resolver';
import type Resolver from 'ember-resolver';

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

Expand All @@ -13,6 +13,7 @@ export interface Owner
ContainerProxyMixin,
RegistryProxyMixin {
_emberTestHelpersMockOwner?: boolean;
rootElement?: string | Element;

_lookupFactory?(key: string): any;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getContext } from '../setup-context';
import { getContext, isTestContext } from '../setup-context';
import { isDocument, isElement } from './-target';

/**
Expand All @@ -9,14 +9,15 @@ import { isDocument, isElement } from './-target';
*/
export default function getRootElement(): Element | Document {
let context = getContext();
let owner = context && context.owner;

if (!owner) {
if (!context || !isTestContext(context) || !context.owner) {
throw new Error(
'Must setup rendering context before attempting to interact with elements.'
);
}

let owner = context.owner;

let rootElement;
// When the host app uses `setApplication` (instead of `setResolver`) the owner has
// a `rootElement` set on it with the element or id to be used
Expand Down
3 changes: 1 addition & 2 deletions addon-test-support/@ember/test-helpers/dom/tab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ function compileFocusAreas(root: Element = document.body) {
? NodeFilter.FILTER_ACCEPT
: NodeFilter.FILTER_SKIP;
},
},
false
}
);

let node: Node | null;
Expand Down
2 changes: 1 addition & 1 deletion addon-test-support/@ember/test-helpers/dom/tap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export default function tap(
return fireEvent(element, 'touchstart', options)
.then((touchstartEv) =>
fireEvent(element as Element, 'touchend', options).then(
(touchendEv) => [touchstartEv, touchendEv]
(touchendEv) => [touchstartEv, touchendEv] as const
)
)
.then(([touchstartEv, touchendEv]) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ export function __triggerKeyEvent__(
};
} else if (typeof key === 'string' && key.length !== 0) {
let firstCharacter = key[0];
if (firstCharacter !== firstCharacter.toUpperCase()) {
if (!firstCharacter || firstCharacter !== firstCharacter.toUpperCase()) {
throw new Error(
`Must provide a \`key\` to \`triggerKeyEvent\` that starts with an uppercase character but you passed \`${key}\`.`
);
Expand Down
2 changes: 1 addition & 1 deletion addon-test-support/@ember/test-helpers/dom/type-in.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ function fillOut(
.map((character) => keyEntry(element, character));
return inputFunctions.reduce((currentPromise, func) => {
return currentPromise.then(() => delayedExecute(delay)).then(func);
}, Promise.resolve(undefined));
}, Promise.resolve());
}

// eslint-disable-next-line require-jsdoc
Expand Down
7 changes: 6 additions & 1 deletion addon-test-support/@ember/test-helpers/has-ember-version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ import Ember from 'ember';
@returns {boolean} true if the Ember version is >= MAJOR.MINOR specified, false otherwise
*/
export default function hasEmberVersion(major: number, minor: number): boolean {
let numbers = Ember.VERSION.split('-')[0].split('.');
let numbers = Ember.VERSION.split('-')[0]?.split('.');

if (!numbers || !numbers[0] || !numbers[1]) {
throw new Error('`Ember.VERSION` is not set.');
}

let actualMajor = parseInt(numbers[0], 10);
let actualMinor = parseInt(numbers[1], 10);
return actualMajor > major || (actualMajor === major && actualMinor >= minor);
Expand Down
15 changes: 15 additions & 0 deletions addon-test-support/@ember/test-helpers/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
import {
Backburner,
DeferredActionQueues,
} from '@ember/runloop/-private/backburner';

export { setResolver, getResolver } from './resolver';
export { getApplication, setApplication } from './application';
export {
Expand Down Expand Up @@ -54,3 +59,13 @@ export { default as find } from './dom/find';
export { default as findAll } from './dom/find-all';
export { default as typeIn } from './dom/type-in';
export { default as scrollTo } from './dom/scroll-to';

// Declaration-merge for our internal purposes.
declare module '@ember/runloop' {
interface PrivateBackburner extends Backburner {
hasTimers(): boolean;
currentInstance: DeferredActionQueues | null;
}

export const _backburner: PrivateBackburner;
}
2 changes: 1 addition & 1 deletion addon-test-support/@ember/test-helpers/resolver.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type Resolver from '@ember/application/resolver';
import type Resolver from 'ember-resolver';

let __resolver__: Resolver | undefined;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import hasEmberVersion from './has-ember-version';
import settled from './settled';
import getTestMetadata, { ITestMetadata } from './test-metadata';
import { runHooks } from './-internal/helper-hooks';
import { Router } from '@ember/routing';

export interface ApplicationTestContext extends TestContext {
element?: Element | null;
Expand Down Expand Up @@ -74,7 +75,7 @@ export function hasPendingTransitions(): boolean | null {
*/
export function setupRouterSettlednessTracking() {
const context = getContext();
if (context === undefined) {
if (context === undefined || !isTestContext(context)) {
throw new Error(
'Cannot setupRouterSettlednessTracking outside of a test context'
);
Expand All @@ -87,17 +88,17 @@ export function setupRouterSettlednessTracking() {
HAS_SETUP_ROUTER.set(context, true);

let { owner } = context;
let router;
let router: Router;
if (CAN_USE_ROUTER_EVENTS) {
router = owner.lookup('service:router');
router = owner.lookup('service:router') as Router;

// track pending transitions via the public routeWillChange / routeDidChange APIs
// routeWillChange can fire many times and is only useful to know when we have _started_
// transitioning, we can then use routeDidChange to signal that the transition has settled
router.on('routeWillChange', () => (routerTransitionsPending = true));
router.on('routeDidChange', () => (routerTransitionsPending = false));
} else {
router = owner.lookup('router:main');
router = owner.lookup('router:main') as Router;
ROUTER.set(context, router);
}

Expand Down Expand Up @@ -173,7 +174,7 @@ export function currentRouteName(): string {

let router = context.owner.lookup('router:main');

return get(router, 'currentRouteName');
return get(router, 'currentRouteName') as string;
}

const HAS_CURRENT_URL_ON_ROUTER = hasEmberVersion(2, 13);
Expand All @@ -193,9 +194,12 @@ export function currentURL(): string {
let router = context.owner.lookup('router:main');

if (HAS_CURRENT_URL_ON_ROUTER) {
return get(router, 'currentURL');
return get(router, 'currentURL') as string;
} else {
return get(router, 'location').getURL();
// SAFETY: this is *positively ancient* and should probably be removed at
// some point; old routers which don't have `currentURL` *should* have a
// `location` with `getURL()` per the docs for 2.12.
return (get(router, 'location') as any).getURL();
}
}

Expand Down

0 comments on commit 30e9c1f

Please sign in to comment.