Merge pull request #1389 from gitKrystan/fix-canary

Support `@types`, preview types, and stable types for Ember.js

Author: chriskrycho

.github/workflows/ci-build.yml

@@ -4,10 +4,10 @@ on:
   push:
     branches:
       - master
-      - 'v*'
+      - "v*"
   pull_request:
   schedule:
-    - cron: '0 3 * * *' # daily, at 3am
+    - cron: "0 3 * * *" # daily, at 3am
 
 jobs:
   test:
@@ -24,7 +24,7 @@ jobs:
 
   floating-dependencies:
     timeout-minutes: 10
-    name: 'Floating Dependencies'
+    name: "Floating Dependencies"
     runs-on: ubuntu-latest
 
     steps:
@@ -35,7 +35,7 @@ jobs:
 
   try-scenarios:
     timeout-minutes: 10
-    name: 'Try: ${{ matrix.ember-try-scenario }}'
+    name: "Try: ${{ matrix.ember-try-scenario }}"
 
     runs-on: ubuntu-latest
 
@@ -47,7 +47,7 @@ jobs:
         ember-try-scenario:
           - ember-lts-4.4
           - ember-lts-4.8
-          - ember-lts-5.0
+          - ember-lts-4.12
           - ember-release
           - ember-beta
           - ember-canary
@@ -63,6 +63,9 @@ jobs:
       - name: test
         working-directory: addon
         run: node_modules/.bin/ember try:one ${{ matrix.ember-try-scenario }} --skip-cleanup
+      - name: types compatibility
+        working-directory: addon
+        run: yarn lint:ts
 
   types:
     runs-on: ubuntu-latest

addon/.eslintrc.js

@@ -55,10 +55,6 @@ module.exports = {
     {
       files: ['addon-test-support/**/*.[jt]s'],
       plugins: ['disable-features'],
-      rules: {
-        'disable-features/disable-async-await': 'error',
-        'disable-features/disable-generator-functions': 'error',
-      },
     },
     {
       files: ['addon-test-support/**/*.[jt]s'],

addon/addon-test-support/@ember/test-helpers/-internal/-owner-mixin-imports.ts

@@ -0,0 +1,78 @@
+// These utter shenanigans allow this to work regardless of what types you are
+// importing from. The idea is: you supply multiple imports, each of which is
+// mutually exclusive with the others in terms of valid imports, then ignore the
+// imports so they type check, then "resolve" between them based on which one is
+// actually defined. 😭
+//
+// TODO: at the next major version (4.0), when we drop support for Ember < 5.1
+// or some later version and can therefore either depend *somewhat* more on
+// these import locations *or* can drop them entirely (depending on the path
+// taken in terms of the `Owner` stack), switch to the normal imports:
+// ```
+// import {
+//   ContainerProxyMixin,
+//   RegistryProxyMixin,
+// } from '@ember/-internals/runtime';
+// ```
+//
+// For now, we are stuck with this utterly horrifying hack to get the types for
+// these reliably.
+
+// This pair of types lets us check whether the imported type is set. We can
+// then use that to pick between the imported types. The first type produces
+// `any` when the type is otherwise not defined at all, which means we cannot
+// use it directly because `any` will fall through, well, basically *any* check
+// we might come up with... other than checking whether `any extends` it. The
+// second uses that to get back a `never` so we can then "pick" between the
+// imported types.
+type AnyIfNeverElseT<T> = T extends never ? never : T;
+type IsNever<T> = any extends AnyIfNeverElseT<T> ? never : T;
+
+// This mapped type does the "picking". You can pass it any number of types,
+// where only one should be available, and it will "resolve" that type.
+// Constraint: the `Array` type must include mutually exclusive types, such that
+// it should be impossible to ever get back more than one; otherwise, you will
+// end up with a union type of the two. That is very unlikely to be what you
+// want!
+type NonNever<Types extends Array<unknown>> = {
+  [Index in keyof Types]: IsNever<Types[Index]>;
+}[number];
+
+// Imports from `@types`
+// @ts-ignore
+import type CPM_DTS from '@ember/engine/-private/container-proxy-mixin.d.ts';
+// @ts-ignore
+import type RPM_DTS from '@ember/engine/-private/registry-proxy-mixin.d.ts';
+
+// Imports from the preview types on 4.8
+// @ts-ignore
+import type CPM_4_8 from '@ember/engine/-private/container-proxy-mixin';
+// @ts-ignore
+import type RPM_4_8 from '@ember/engine/-private/registry-proxy-mixin';
+
+// Imports from the preview types on 4.12
+// @ts-ignore
+import type CPM_4_12 from '@ember/-internals/runtime/lib/mixins/container_proxy';
+// @ts-ignore
+import type RPM_4_12 from '@ember/-internals/runtime/lib/mixins/registry_proxy';
+
+// Imports available in the stable types
+// @ts-ignore
+import type { ContainerProxyMixin as CPM_stable } from '@ember/-internals/runtime';
+// @ts-ignore
+import type { RegistryProxyMixin as RPM_stable } from '@ember/-internals/runtime';
+
+// We also resolve the *values* from a "stable" location. However, the *types*
+// for the Ember namespace do not consistently include this definition (they do
+// on the stable types, but not in the preview or DT types), so cast as `any` so
+// that it resolves regardless.
+import Ember from 'ember';
+export const ContainerProxyMixin = (Ember as any)._ContainerProxyMixin;
+export type ContainerProxyMixin = NonNever<
+  [CPM_DTS, CPM_4_8, CPM_4_12, CPM_stable]
+>;
+
+export const RegistryProxyMixin = (Ember as any)._RegistryProxyMixin;
+export type RegistryProxyMixin = NonNever<
+  [RPM_DTS, RPM_4_8, RPM_4_12, RPM_stable]
+>;

addon/addon-test-support/@ember/test-helpers/-internal/build-registry.ts

@@ -6,6 +6,16 @@ import EmberObject from '@ember/object';
 import require, { has } from 'require';
 import Ember from 'ember';
 
+import { FullName } from '@ember/owner';
+
+// These shenanigans work around the fact that the import locations are not
+// public API and are not stable, so we jump through hoops to get the right
+// types and values to use.
+import {
+  ContainerProxyMixin,
+  RegistryProxyMixin,
+} from './-owner-mixin-imports';
+
 /**
  * Adds methods that are normally only on registry to the container. This is largely to support the legacy APIs
  * that are not using `owner` (but are still using `this.container`).
@@ -40,12 +50,14 @@ function exposeRegistryMethodsWithoutDeprecations(container: any) {
   }
 }
 
-const RegistryProxyMixin = (Ember as any)._RegistryProxyMixin;
-const ContainerProxyMixin = (Ember as any)._ContainerProxyMixin;
-
+// NOTE: this is the same as what `EngineInstance`/`ApplicationInstance`
+// implement, and is thus a superset of the `InternalOwner` contract from Ember
+// itself.
+interface Owner extends RegistryProxyMixin, ContainerProxyMixin {}
 const Owner = EmberObject.extend(RegistryProxyMixin, ContainerProxyMixin, {
   _emberTestHelpersMockOwner: true,
 
+  /* eslint-disable valid-jsdoc */
   /**
    * Unregister a factory and its instance.
    *
@@ -57,13 +69,16 @@ const Owner = EmberObject.extend(RegistryProxyMixin, ContainerProxyMixin, {
    * @see {@link https://github.com/emberjs/ember.js/pull/12680}
    * @see {@link https://github.com/emberjs/ember.js/blob/v4.5.0-alpha.5/packages/%40ember/engine/instance.ts#L152-L167}
    */
-  unregister(fullName: string) {
-    // @ts-expect-error
-    this['__container__'].reset(fullName);
+  /* eslint-enable valid-jsdoc */
+  unregister(this: Owner, fullName: FullName) {
+    // SAFETY: this is always present, but only the stable type definitions from
+    // Ember actually preserve it, since it is private API.
+    (this as any)['__container__'].reset(fullName);
 
     // We overwrote this method from RegistryProxyMixin.
-    // @ts-expect-error
-    this['__registry__'].unregister(fullName);
+    // SAFETY: this is always present, but only the stable type definitions from
+    // Ember actually preserve it, since it is private API.
+    (this as any)['__registry__'].unregister(fullName);
   },
 });
 
@@ -72,46 +87,62 @@ const Owner = EmberObject.extend(RegistryProxyMixin, ContainerProxyMixin, {
  * @param {Object} resolver the resolver to use with the registry
  * @returns {Object} owner, container, registry
  */
-export default function (resolver: Resolver) {
-  let fallbackRegistry, registry, container;
-  let namespace = EmberObject.create({
-    // @ts-expect-error
-    Resolver: {
-      create() {
-        return resolver;
-      },
+export default function buildRegistry(resolver: Resolver) {
+  let namespace = new Application();
+  // @ts-ignore: this is actually the correcct type, but there was a typo in
+  // Ember's docs for many years which meant that there was a matching problem
+  // in the types for Ember's definition of `Engine`. Once we require at least
+  // Ember 5.1 (in some future breaking change), this ts-ignore can be removed.
+  namespace.Resolver = {
+    create() {
+      return resolver;
     },
-  });
+  };
 
-  fallbackRegistry = (Application as any).buildRegistry(namespace);
+  // @ts-ignore: this is private API.
+  let fallbackRegistry = Application.buildRegistry(namespace);
   // TODO: only do this on Ember < 3.13
-  fallbackRegistry.register(
-    'component-lookup:main',
-    (Ember as any).ComponentLookup
-  );
+  // @ts-ignore: this is private API.
+  fallbackRegistry.register('component-lookup:main', Ember.ComponentLookup);
 
-  registry = new (Ember as any).Registry({
+  // @ts-ignore: this is private API.
+  let registry = new Ember.Registry({
     fallback: fallbackRegistry,
   });
 
-  (ApplicationInstance as any).setupRegistry(registry);
+  // @ts-ignore: this is private API.
+  ApplicationInstance.setupRegistry(registry);
 
   // these properties are set on the fallback registry by `buildRegistry`
   // and on the primary registry within the ApplicationInstance constructor
   // but we need to manually recreate them since ApplicationInstance's are not
   // exposed externally
+  // @ts-ignore: this is private API.
   registry.normalizeFullName = fallbackRegistry.normalizeFullName;
+  // @ts-ignore: this is private API.
   registry.makeToString = fallbackRegistry.makeToString;
+  // @ts-ignore: this is private API.
   registry.describe = fallbackRegistry.describe;
 
   let owner = Owner.create({
-    // @ts-expect-error
+    // @ts-ignore -- we do not have type safety for `Object.extend` so the type
+    // of `Owner` here is just `EmberObject`, but we *do* constrain it to allow
+    // only types from the actual class, so these fields are not accepted.
+    // However, we can see that they are valid, based on the definition of
+    // `Owner` above given that it fulfills the `InternalOwner` contract and
+    // also extends it just as `EngineInstance` does internally.
+    //
+    // NOTE: we use an `ignore` directive rather than `expect-error` because in
+    // *some* versions of the types, we *do* have (at least some of) this
+    // safety, and maximal backwards compatibility means we have to account for
+    // that.
     __registry__: registry,
     __container__: null,
-  });
+  }) as unknown as Owner;
 
-  container = registry.container({ owner: owner });
-  // @ts-expect-error
+  // @ts-ignore: this is private API.
+  let container = registry.container({ owner: owner });
+  // @ts-ignore: this is private API.
   owner.__container__ = container;
 
   exposeRegistryMethodsWithoutDeprecations(container);

addon/addon-test-support/@ember/test-helpers/-internal/debug-info.ts

@@ -1,8 +1,8 @@
+// @ts-ignore: this is private API. This import will work Ember 5.1+ since it
+// "provides" this public API, but does not for earlier versions. As a result,
+// this type will be `any`.
 import { _backburner } from '@ember/runloop';
-import {
-  DebugInfo as BackburnerDebugInfo,
-  QueueItem,
-} from '@ember/runloop/-private/backburner';
+import { DebugInfo as BackburnerDebugInfo } from '@ember/runloop/-private/backburner';
 import { DebugInfoHelper, debugInfoHelpers } from './debug-info-helpers';
 import { getPendingWaiterState, PendingWaiterState } from '@ember/test-waiters';
 
@@ -109,26 +109,29 @@ export class TestDebugInfo implements DebugInfo {
 
         this._summaryInfo.pendingScheduledQueueItemCount =
           this._debugInfo.instanceStack
-            .filter((q) => q)
+            .filter(isNotNullable)
             .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;
+              Object.values(item).forEach((queueItems) => {
+                // SAFETY: this cast is required for versions of Ember which do
+                // not supply a correct definition of these types. It should
+                // also be compatible with the version where Ember *does* supply
+                // the types correctly.
+                total +=
+                  (queueItems as Array<unknown> | undefined)?.length ?? 0;
               });
 
               return total;
             }, 0);
         this._summaryInfo.pendingScheduledQueueItemStackTraces =
           this._debugInfo.instanceStack
-            .filter((q) => q)
+            .filter(isNotNullable)
             .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(
+              Object.values(deferredActionQueues).forEach((queueItems) => {
+                // SAFETY: this cast is required for versions of Ember which do
+                // not supply a correct definition of these types. It should
+                // also be compatible with the version where Ember *does* supply
+                // the types correctly.
+                (queueItems as Array<{ stack: string }> | undefined)?.forEach(
                   (queueItem) => queueItem.stack && stacks.push(queueItem.stack)
                 );
               });
@@ -222,3 +225,8 @@ export class TestDebugInfo implements DebugInfo {
     return `${title}: ${count}`;
   }
 }
+
+// eslint-disable-next-line require-jsdoc
+function isNotNullable<T extends {}>(value: T | null | undefined): value is T {
+  return value != null;
+}

addon/addon-test-support/@ember/test-helpers/-internal/is-component.ts

@@ -1,5 +1,8 @@
 import type { ComponentLike } from '@glint/template';
 
+// @ts-ignore: types for this API is not consistently available (via transitive
+// deps) and we do not currently want to make it an explicit dependency. It
+// does, however, consistently work at runtime. :sigh:
 import { getInternalComponentManager as getComponentManager } from '@glimmer/manager';
 
 /**

addon/addon-test-support/@ember/test-helpers/build-owner.ts

@@ -2,19 +2,19 @@ import Application from '@ember/application';
 import type { Resolver } from '@ember/owner';
 
 import legacyBuildRegistry from './-internal/build-registry';
-import ContainerProxyMixin from '@ember/engine/-private/container-proxy-mixin';
-import RegistryProxyMixin from '@ember/engine/-private/registry-proxy-mixin';
-import CoreObject from '@ember/object/core';
-
-export interface Owner
-  extends CoreObject,
-    ContainerProxyMixin,
-    RegistryProxyMixin {
+import EmberOwner from '@ember/owner';
+import { SimpleElement } from '@simple-dom/interface';
+
+export interface Owner extends EmberOwner {
   _emberTestHelpersMockOwner?: boolean;
-  rootElement?: string | Element;
+  rootElement?: string | Element | SimpleElement | null;
 
   _lookupFactory?(key: string): any;
 
+  // Note: this should be the same as `Application['visit']`, but that *type* is
+  // only available from Ember 4.12 on. Once we require Ember >= 5.1 and rely on
+  // the stable types, this will not be necessary and the related `@ts-ignore`
+  // below can also be removed.
   visit(url: string, options?: { [key: string]: any }): Promise<any>;
 }
 
@@ -42,9 +42,11 @@ export default function buildOwner(
   resolver: Resolver | undefined | null
 ): Promise<Owner> {
   if (application) {
-    return application
-      .boot()
-      .then((app) => app.buildInstance().boot()) as unknown as Promise<Owner>;
+    // @ts-ignore: this type is correct and will check against Ember 4.12 or 5.1
+    // or later. However, the first round of preview types in Ember 4.8 does not
+    // include the `visit` API (it was missing for many years!) and therefore
+    // there is no way to make this assignable accross all supported versions.
+    return application.boot().then((app) => app.buildInstance().boot());
   }
 
   if (!resolver) {