Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert ember-testing and @ember/test to TS #20090

Merged
merged 2 commits into from
May 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/@ember/-internals/error-handling/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ export function setOnerror(handler: Function) {
onerror = handler;
}

let dispatchOverride: Function | undefined;
let dispatchOverride: Function | null = null;
wagenet marked this conversation as resolved.
Show resolved Hide resolved

// allows testing adapter to override dispatch
export function getDispatchOverride() {
return dispatchOverride;
}
export function setDispatchOverride(handler: Function) {
export function setDispatchOverride(handler: Function | null) {
dispatchOverride = handler;
}
12 changes: 11 additions & 1 deletion packages/@ember/application/lib/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
} from '@ember/-internals/routing';
import type { BootOptions } from '../instance';
import ApplicationInstance from '../instance';
import Engine from '@ember/engine';
import Engine, { buildInitializerMethod } from '@ember/engine';
import type { Container, Registry } from '@ember/-internals/container';
import { privatize as P } from '@ember/-internals/container';
import { setupApplicationRegistry } from '@ember/-internals/glimmer';
Expand Down Expand Up @@ -220,6 +220,16 @@ class Application extends Engine {
return registry;
}

static initializer = buildInitializerMethod<'initializers', Application>(
'initializers',
'initializer'
);

static instanceInitializer = buildInitializerMethod<'instanceInitializers', ApplicationInstance>(
'instanceInitializers',
'instance initializer'
);
Comment on lines +223 to +231
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I poked at why this is the way it is, and I believe if we update how buildInitializerMethod is defined, we can drop all the casting shenanigans. 😅

// Two significant changes here:
//
// 1. Only parameterize over the bucket name, and simply *infer* the type of
//    initializer from it using the bucket name.
// 2. Make the return type explicit, so that the type parameterization can fall
//    out from it naturally. that the actual returned function can be simpler,
//    not having to do the type parameterization on it.
function buildInitializerMethod<
  B extends 'initializers' | 'instanceInitializers'
>(
  bucketName: B,
  humanName: string
): (this: typeof Engine, initializer: Initializer<Kind<B>>) => void {
  return function (initializer) {
    // ...
  };
}

type Kind<B extends 'initializers' | 'instanceInitializers'> =
  B extends 'initializers' ? Engine : EngineInstance;

Then the usage here will be:

  static initializer = buildInitializerMethod('initializers', 'initializer');

  static instanceInitializer = buildInitializerMethod(
    'instanceInitializers',
    'instance initializer'
  );

playground

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems likely. I didn’t try very hard beyond getting it to work, but it does seem worth trying to clean it up a bit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this works except for one significant problem: I think we want initializers on Application to return Application/ApplicationInstance not Engine/EngineInstance.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. Hmm. 🤔 That’s… actually impossible to guarantee from a types POV given the existing API, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For posterity: We spent some time poking at this and it is indeed impossible to guarantee the correct behavior here… because, and only because, it requires inference on the this type, and that interacts badly with Object.prototype.call. 😩 Net, we'll leave it as is and… hope for a day when initializers are dead?


/**
The root DOM element of the Application. This can be specified as an
element or a [selector string](https://developer.mozilla.org/en-US/docs/Learn/CSS/Building_blocks/Selectors#reference_table_of_selectors).
Expand Down
2 changes: 1 addition & 1 deletion packages/@ember/application/lib/lazy_load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export let _loaded = loaded;
@param callback {Function} callback to be called
@private
*/
export function onLoad(name: string, callback: (obj: unknown) => void) {
export function onLoad(name: string, callback: (obj: any) => void) {
let object = loaded[name];

let hooks = (loadHooks[name] ??= []);
Expand Down
2 changes: 1 addition & 1 deletion packages/@ember/debug/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export type AssertFunc = (desc: string, condition?: unknown) => asserts conditio
export type DebugFunc = (message: string) => void;
export type DebugSealFunc = (obj: object) => void;
export type DebugFreezeFunc = (obj: object) => void;
export type InfoFunc = (message: string, options: object) => void;
export type InfoFunc = (message: string, options?: object) => void;
export type RunInDebugFunc = (func: () => void) => void;
export type DeprecateFuncFunc = (
message: string,
Expand Down
3 changes: 2 additions & 1 deletion packages/@ember/engine/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,8 @@ function resolverFor(namespace: Engine) {
return ResolverClass.create(props);
}

function buildInitializerMethod<
/** @internal */
export function buildInitializerMethod<
Comment on lines +470 to +471
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per comments above, we should update this to be smarter about how it does its thing.

B extends 'initializers' | 'instanceInitializers',
T extends B extends 'initializers' ? Engine : EngineInstance
>(bucketName: B, humanName: string) {
Expand Down
3 changes: 0 additions & 3 deletions packages/@ember/test/adapter.js

This file was deleted.

3 changes: 3 additions & 0 deletions packages/@ember/test/adapter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { Adapter } from 'ember-testing';

export default Adapter;
11 changes: 6 additions & 5 deletions packages/@ember/test/index.js → packages/@ember/test/index.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import require, { has } from 'require';
import { type Test } from 'ember-testing';

export let registerAsyncHelper;
export let registerHelper;
export let registerWaiter;
export let unregisterHelper;
export let unregisterWaiter;
export let registerAsyncHelper: typeof Test['registerAsyncHelper'] | (() => never);
export let registerHelper: typeof Test['registerHelper'] | (() => never);
export let registerWaiter: typeof Test['registerWaiter'] | (() => never);
export let unregisterHelper: typeof Test['unregisterHelper'] | (() => never);
export let unregisterWaiter: typeof Test['unregisterWaiter'] | (() => never);

if (has('ember-testing')) {
let { Test } = require('ember-testing');
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ import { Object as EmberObject } from '@ember/-internals/runtime';
@class TestAdapter
@public
*/
export default EmberObject.extend({
interface Adapter extends EmberObject {
asyncStart(): void;
asyncEnd(): void;
exception(error: unknown): never;
}
const Adapter = EmberObject.extend({
/**
This callback will be called whenever an async operation is about to start.

Expand Down Expand Up @@ -48,7 +53,9 @@ export default EmberObject.extend({
@method exception
@param {String} error The exception to be raised.
*/
exception(error) {
exception(error: unknown) {
throw error;
},
});

export default Adapter;
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@

import { inspect } from '@ember/-internals/utils';
import Adapter from './adapter';

interface VeryOldQunit {
stop(): void;
}

function isVeryOldQunit(obj: unknown): obj is VeryOldQunit {
return obj != null && typeof (obj as VeryOldQunit).stop === 'function';
}

/**
@module ember
*/
Expand All @@ -14,25 +23,28 @@ import Adapter from './adapter';
@extends TestAdapter
@public
*/
export default Adapter.extend({
// eslint-disable-next-line @typescript-eslint/no-empty-interface
interface QUnitAdapter extends Adapter {}
const QUnitAdapter = Adapter.extend({
init() {
this.doneCallbacks = [];
},

asyncStart() {
if (typeof QUnit.stop === 'function') {
if (isVeryOldQunit(QUnit)) {
// very old QUnit version
// eslint-disable-next-line qunit/no-qunit-stop
QUnit.stop();
} else {
this.doneCallbacks.push(QUnit.config.current ? QUnit.config.current.assert.async() : null);
}
},

asyncEnd() {
// checking for QUnit.stop here (even though we _need_ QUnit.start) because
// QUnit.start() still exists in QUnit 2.x (it just throws an error when calling
// inside a test context)
if (typeof QUnit.stop === 'function') {
if (isVeryOldQunit(QUnit)) {
QUnit.start();
} else {
let done = this.doneCallbacks.pop();
Expand All @@ -42,7 +54,10 @@ export default Adapter.extend({
}
}
},
exception(error) {

exception(error: unknown) {
QUnit.config.current.assert.ok(false, inspect(error));
},
});

export default QUnitAdapter;
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,19 @@ import TestPromise, { resolve, getLastPromise } from '../test/promise';
import run from '../test/run';
import { invokeInjectHelpersCallbacks } from '../test/on_inject_helpers';
import { asyncStart, asyncEnd } from '../test/adapter';
import type Application from '@ember/application';
import type { AnyFn } from '@ember/-internals/utils/types';
import { assert } from '@ember/debug';

export interface TestableApp extends Application {
testing?: boolean;
testHelpers: Record<string, (...args: unknown[]) => unknown>;
originalMethods: Record<string, (...args: unknown[]) => unknown>;
setupForTesting(): void;
helperContainer: object | null;
injectTestHelpers(helperContainer: unknown): void;
removeTestHelpers(): void;
}

EmberApplication.reopen({
/**
Expand Down Expand Up @@ -105,25 +118,28 @@ EmberApplication.reopen({
@method injectTestHelpers
@public
*/
injectTestHelpers(helperContainer) {
injectTestHelpers(this: TestableApp, helperContainer: object) {
if (helperContainer) {
this.helperContainer = helperContainer;
} else {
this.helperContainer = window;
}

this.reopen({
willDestroy() {
willDestroy(this: TestableApp) {
this._super(...arguments);
this.removeTestHelpers();
},
});

this.testHelpers = {};
for (let name in helpers) {
this.originalMethods[name] = this.helperContainer[name];
this.testHelpers[name] = this.helperContainer[name] = helper(this, name);
protoWrap(TestPromise.prototype, name, helper(this, name), helpers[name].meta.wait);
// SAFETY: It is safe to access a property on an object
this.originalMethods[name] = (this.helperContainer as any)[name];
// SAFETY: It is not quite as safe to do this, but it _seems_ to be ok.
this.testHelpers[name] = (this.helperContainer as any)[name] = helper(this, name);
// SAFETY: We checked that it exists
protoWrap(TestPromise.prototype, name, helper(this, name), helpers[name]!.meta.wait);
}

invokeInjectHelpersCallbacks(this);
Expand All @@ -149,7 +165,8 @@ EmberApplication.reopen({

for (let name in helpers) {
this.helperContainer[name] = this.originalMethods[name];
delete TestPromise.prototype[name];
// SAFETY: This is a weird thing, but it's not technically unsafe here.
delete (TestPromise.prototype as any)[name];
delete this.testHelpers[name];
delete this.originalMethods[name];
}
Expand All @@ -159,26 +176,30 @@ EmberApplication.reopen({
// This method is no longer needed
// But still here for backwards compatibility
// of helper chaining
function protoWrap(proto, name, callback, isAsync) {
proto[name] = function (...args) {
function protoWrap(proto: TestPromise<unknown>, name: string, callback: AnyFn, isAsync: boolean) {
// SAFETY: This isn't entirely safe, but it _seems_ to be ok.
(proto as any)[name] = function (...args: unknown[]) {
if (isAsync) {
return callback.apply(this, args);
} else {
return this.then(function () {
// SAFETY: This is not actually safe.
return (this as any).then(function (this: any) {
return callback.apply(this, args);
});
}
};
}

function helper(app, name) {
let fn = helpers[name].method;
let meta = helpers[name].meta;
function helper(app: TestableApp, name: string) {
let helper = helpers[name];
assert(`[BUG] Missing helper: ${name}`, helper);
let fn = helper.method;
let meta = helper.meta;
if (!meta.wait) {
return (...args) => fn.apply(app, [app, ...args]);
return (...args: unknown[]) => fn.apply(app, [app, ...args]);
}

return (...args) => {
return (...args: unknown[]) => {
let lastPromise = run(() => resolve(getLastPromise()));

// wait for last helper's promise to resolve and then
Expand Down
19 changes: 0 additions & 19 deletions packages/ember-testing/lib/ext/rsvp.js

This file was deleted.

22 changes: 22 additions & 0 deletions packages/ember-testing/lib/ext/rsvp.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { RSVP } from '@ember/-internals/runtime';
import { _backburner } from '@ember/runloop';
import { isTesting } from '@ember/debug';
import { asyncStart, asyncEnd } from '../test/adapter';

RSVP.configure(
'async',
function (callback: (promise: Promise<unknown>) => void, promise: Promise<unknown>) {
// if schedule will cause autorun, we need to inform adapter
if (isTesting() && !_backburner.currentInstance) {
asyncStart();
_backburner.schedule('actions', () => {
asyncEnd();
callback(promise);
});
} else {
_backburner.schedule('actions', () => callback(promise));
}
}
);

export default RSVP;
3 changes: 0 additions & 3 deletions packages/ember-testing/lib/helpers/and_then.js

This file was deleted.

8 changes: 8 additions & 0 deletions packages/ember-testing/lib/helpers/and_then.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { assert } from '@ember/debug';
import type { TestableApp } from '../ext/application';

export default function andThen(app: TestableApp, callback: (app: TestableApp) => unknown) {
let wait = app.testHelpers['wait'];
assert('[BUG] Missing wait helper', wait);
return wait(callback(app));
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
@module ember
*/
import { get } from '@ember/-internals/metal';
import { RoutingService } from '@ember/-internals/routing';
import type Application from '@ember/application';
import { assert } from '@ember/debug';

/**
Returns the current path.
Expand All @@ -21,7 +24,14 @@ click('#some-link-id').then(validateURL);
@since 1.5.0
@public
*/
export default function currentPath(app) {
export default function currentPath(app: Application) {
assert('[BUG] app.__container__ is not set', app.__container__);

let routingService = app.__container__.lookup('service:-routing');
assert(
'[BUG] service:-routing is not a RoutingService',
routingService instanceof RoutingService
);

return get(routingService, 'currentPath');
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
@module ember
*/
import { get } from '@ember/-internals/metal';
import { RoutingService } from '@ember/-internals/routing';
import type Application from '@ember/application';
import { assert } from '@ember/debug';
/**
Returns the currently active route name.

Expand All @@ -19,7 +22,14 @@ visit('/some/path').then(validateRouteName)
@since 1.5.0
@public
*/
export default function currentRouteName(app) {
export default function currentRouteName(app: Application) {
assert('[BUG] app.__container__ is not set', app.__container__);

let routingService = app.__container__.lookup('service:-routing');
assert(
'[BUG] service:-routing is not a RoutingService',
routingService instanceof RoutingService
);

return get(routingService, 'currentRouteName');
}
Loading