Merge pull request #386 from glimmerjs/backport-component-signature

Backport component signature (#385) to v1.x

.github/workflows/ci.yml

@@ -18,7 +18,6 @@ jobs:
       - run: yarn install --frozen-lockfile --non-interactive
       - run: yarn tslint
       - run: yarn build
-      - run: yarn dtslint --installAll
       - run: yarn test:types
 
   ember-version-tests:

package.json

@@ -10,7 +10,8 @@
     "test": "ember test && ember test --env production",
     "test:node": "bin/run-node-tests.js",
     "test:ember": "yarn workspace @glimmer/component ember try:one",
-    "test:types": "dtslint test/types",
+    "test:types": "tsc --version && tsc --noEmit --project test/types",
+    "test:watch": "testem",
     "tslint": "tslint --project tsconfig.json",
     "problems": "tsc -p tsconfig.json --noEmit"
   },
@@ -39,6 +40,7 @@
     "ember-cli": "^3.7.1",
     "ember-cli-sauce": "^1.3.0",
     "eslint": "^4.3.0",
+    "expect-type": "0.11",
     "glob": "^7.0.5",
     "lerna-changelog": "^0.7.0",
     "loader.js": "^4.0.10",
@@ -60,7 +62,6 @@
     "typescript": "~3.5.3"
   },
   "dependencies": {
-    "dtslint": "^1.0.3",
     "tslib": "^1.8.0"
   },
   "resolutions": {
@@ -77,4 +78,4 @@
       "deprecation": ":warning: Deprecation"
     }
   }
-}
+}

packages/@glimmer/application/src/components/component-managers/custom.ts

@@ -41,11 +41,7 @@ export const CAPABILITIES: ComponentCapabilities = {
   dynamicScope: true,
 };
 
-export interface Capabilities {
-  asyncLifecycleCallbacks: boolean;
-  destructor: boolean;
-  updateHook: boolean;
-}
+export { Capabilities };
 
 export interface Args {
   named: Dict<unknown>;

packages/@glimmer/application/src/references.ts

@@ -163,6 +163,9 @@ export class RootPropertyReference<T> extends PropertyReference<T> {
     consume(tag);
     update(this.tag, tag);
 
+    // This is a type error in recent versions of TS, but/and it should be
+    // resolved on the v2 branch, rather than against v1.x.
+    // @ts-ignore
     return value;
   }
 }
@@ -180,6 +183,8 @@ export class NestedPropertyReference<T> extends PropertyReference<T> {
     this.tag = combine([parentReferenceTag, propertyTag]);
   }
 
+  // This is patently nonsense, but/and it is resolved on the v2 branch.
+  // @ts-ignore
   compute() {
     let { parentReference, propertyTag, propertyKey } = this;
 

packages/@glimmer/component/addon/-private/component.ts

@@ -2,12 +2,89 @@ import { DEBUG } from '@glimmer/env';
 import { setOwner } from './owner';
 import { isDestroying, isDestroyed } from './destroyables';
 
-export let ARGS_SET: WeakMap<any, boolean>;
+// This provides a type-safe `WeakMap`: the getter and setter link the key to a
+// specific value. This is how `WeakMap`s actually behave, but the TS type
+// system does not (yet!) have a good way to capture that for types like
+// `WeakMap` where the type is generic over another generic type (here, `Args`).
+interface ArgsSetMap extends WeakMap<Args<unknown>, boolean> {
+  get<S>(key: Args<S>): boolean | undefined;
+  set<S>(key: Args<S>, value: boolean): this;
+}
+
+// SAFETY: this only holds because we *only* acces this when `DEBUG` is `true`.
+// There is not a great way to connect that data in TS at present.
+export let ARGS_SET: ArgsSetMap;
 
 if (DEBUG) {
-  ARGS_SET = new WeakMap();
+  ARGS_SET = new WeakMap() as ArgsSetMap;
 }
 
+// --- Type utilities for component signatures --- //
+// Type-only "symbol" to use with `EmptyObject` below, so that it is *not*
+// equivalent to an empty interface.
+declare const Empty: unique symbol;
+
+/**
+ * This provides us a way to have a "fallback" which represents an empty object,
+ * without the downsides of how TS treats `{}`. Specifically: this will
+ * correctly leverage "excess property checking" so that, given a component
+ * which has no named args, if someone invokes it with any named args, they will
+ * get a type error.
+ *
+ * @internal This is exported so declaration emit works (if it were not emitted,
+ *   declarations which fall back to it would not work). It is *not* intended for
+ *   public usage, and the specific mechanics it uses may change at any time.
+ *   The location of this export *is* part of the public API, because moving it
+ *   will break existing declarations, but is not legal for end users to import
+ *   themselves, so ***DO NOT RELY ON IT***.
+ */
+export type EmptyObject = { [Empty]?: true };
+
+type GetOrElse<Obj, K extends PropertyKey, Fallback> = Obj extends { [Key in K]: infer U }
+  ? U
+  : Fallback;
+
+/** Given a signature `S`, get back the `Args` type. */
+type ArgsFor<S> = S extends { Args: infer Args }
+  ? Args extends { Named?: object; Positional?: unknown[] } // Are they longhand already?
+    ? {
+        Named: GetOrElse<S['Args'], 'Named', EmptyObject>;
+        Positional: GetOrElse<S['Args'], 'Positional', []>;
+      }
+    : { Named: S['Args']; Positional: [] }
+  : { Named: EmptyObject; Positional: [] };
+
+/**
+ * Given any allowed shorthand form of a signature, desugars it to its full
+ * expanded type.
+ *
+ * @internal This is only exported so we can avoid duplicating it in
+ *   [Glint](https://github.com/typed-ember/glint) or other such tooling. It is
+ *   *not* intended for public usage, and the specific mechanics it uses may
+ *   change at any time. Although the signature produced by is part of Glimmer's
+ *   public API the existence and mechanics of this specific symbol are *not*,
+ *   so ***DO NOT RELY ON IT***.
+ */
+export type ExpandSignature<T> = {
+  Element: GetOrElse<T, 'Element', null>;
+  Args: keyof T extends 'Args' | 'Element' | 'Blocks' // Is this a `Signature`?
+    ? ArgsFor<T> // Then use `Signature` args
+    : { Named: T; Positional: [] }; // Otherwise fall back to classic `Args`.
+  Blocks: T extends { Blocks: infer Blocks }
+    ? {
+        [Block in keyof Blocks]: Blocks[Block] extends unknown[]
+          ? { Positional: Blocks[Block] }
+          : Blocks[Block];
+      }
+    : EmptyObject;
+};
+
+/**
+ * @internal we use this type for convenience internally; inference means users
+ *   should not normally need to name it
+ */
+export type Args<S> = ExpandSignature<S>['Args']['Named'];
+
 /**
  * The `Component` class defines an encapsulated UI element that is rendered to
  * the DOM. A component is made up of a template and, optionally, this component
@@ -131,7 +208,7 @@ if (DEBUG) {
  * `args` property. For example, if `{{@firstName}}` is `Tom` in the template,
  * inside the component `this.args.firstName` would also be `Tom`.
  */
-export default class BaseComponent<T = object> {
+export default class BaseComponent<S = unknown> {
   /**
    * Constructs a new component and assigns itself the passed properties. You
    * should not construct new components yourself. Instead, Glimmer will
@@ -140,12 +217,10 @@ export default class BaseComponent<T = object> {
    * @param owner
    * @param args
    */
-  constructor(owner: unknown, args: T) {
+  constructor(owner: unknown, args: Args<S>) {
     if (DEBUG && !(owner !== null && typeof owner === 'object' && ARGS_SET.has(args))) {
       throw new Error(
-        `You must pass both the owner and args to super() in your component: ${
-          this.constructor.name
-        }. You can pass them directly, or use ...arguments to pass all arguments through.`
+        `You must pass both the owner and args to super() in your component: ${this.constructor.name}. You can pass them directly, or use ...arguments to pass all arguments through.`
       );
     }
 
@@ -177,18 +252,18 @@ export default class BaseComponent<T = object> {
    * <p>Welcome, {{@firstName}} {{@lastName}}!</p>
    * ```
    */
-  args: Readonly<T>;
+  args: Readonly<Args<S>>;
 
-  get isDestroying() {
+  get isDestroying(): boolean {
     return isDestroying(this);
   }
 
-  get isDestroyed() {
+  get isDestroyed(): boolean {
     return isDestroyed(this);
   }
 
   /**
    * Called before the component has been removed from the DOM.
    */
-  willDestroy() {}
+  willDestroy(): void {}
 }

packages/@glimmer/component/addon/index.ts

@@ -1,10 +1,25 @@
 import { DEBUG } from '@glimmer/env';
-import ApplicationInstance from '@ember/application/instance';
+import type ApplicationInstance from '@ember/application/instance';
 import { setComponentManager } from '@ember/component';
 import { gte } from 'ember-compatibility-helpers';
 
+// Hax because the old version of `@types/ember__component` the `1.x` branch
+// uses does not provide any types for `setComponentManager` *and* because we
+// are using a very old version of `setComponentManager` for versions before
+// Ember 3.8.
+declare module '@ember/component' {
+  // The modern version.
+  export function setComponentManager<T extends object>(
+    factory: (owner: ApplicationInstance) => GlimmerComponentManager,
+    componentClass: T
+  ): T;
+
+  // The pre-3.8 version.
+  export function setComponentManager<T extends object>(name: string, componentClass: T): T;
+}
+
 import GlimmerComponentManager from './-private/ember-component-manager';
-import GlimmerComponentBase from './-private/component';
+import GlimmerComponentBase, { Args } from './-private/component';
 
 let GlimmerComponent = GlimmerComponentBase;
 
@@ -13,15 +28,19 @@ if (DEBUG) {
 
   // TODO: Add GlimmerComponent API docs link to these messages once API docs are live
   function throwMethodUseError(methodName: string) {
-    throw new Error(`You attempted to define the '${methodName}' method on a Glimmer Component, but that lifecycle hook does not exist in Ember.js applications, it only exists in Glimmer.js apps. You can rename this method, and you can trigger it using a modifier such as {{did-insert}} from '@ember/render-modifiers': https://github.com/emberjs/ember-render-modifiers.`);
+    throw new Error(
+      `You attempted to define the '${methodName}' method on a Glimmer Component, but that lifecycle hook does not exist in Ember.js applications, it only exists in Glimmer.js apps. You can rename this method, and you can trigger it using a modifier such as {{did-insert}} from '@ember/render-modifiers': https://github.com/emberjs/ember-render-modifiers.`
+    );
   }
 
   function throwPropertyUseError(propertyName: string) {
-    throw new Error(`You attempted to access the '${propertyName}' property on a Glimmer Component, but that property does not exist in Ember.js applications, it only exists in Glimmer.js apps. You define a class field with the same name on your component class and it will overwrite this error message, but it will not be used by the framework.`);
+    throw new Error(
+      `You attempted to access the '${propertyName}' property on a Glimmer Component, but that property does not exist in Ember.js applications, it only exists in Glimmer.js apps. You define a class field with the same name on your component class and it will overwrite this error message, but it will not be used by the framework.`
+    );
   }
 
-  GlimmerComponent = class GlimmerDebugComponent<T> extends GlimmerComponent<T> {
-    constructor(owner: unknown, args: T) {
+  GlimmerComponent = class GlimmerDebugComponent<S = unknown> extends GlimmerComponent<S> {
+    constructor(owner: unknown, args: Args<S>) {
       super(owner, args);
 
       if (typeof this['didInsertElement'] === 'function') {
@@ -34,9 +53,13 @@ if (DEBUG) {
     }
   };
 
-  let proto = GlimmerComponent.prototype as any;
+  let proto = GlimmerComponent.prototype;
 
-  function defineErrorProp(proto, key, getterMethod) {
+  function defineErrorProp(
+    proto: GlimmerComponentBase,
+    key: string,
+    getterMethod: (key: string) => unknown
+  ) {
     Object.defineProperty(proto, key, {
       get: () => getterMethod(key),
       set(value) {

packages/@glimmer/component/package.json

@@ -93,4 +93,4 @@
     "defaultBlueprint": "install-glimmer-component",
     "main": "ember-addon-main.js"
   }
-}
+}

packages/@glimmer/component/src/component.ts

@@ -1,15 +1,15 @@
 import { assert } from '@glimmer/util';
 import { setComponentManager } from '@glimmer/application';
-import BaseComponent from '../addon/-private/component';
+import BaseComponent, { Args } from '../addon/-private/component';
 import GlimmerComponentManager from './component-manager';
 
 export interface Bounds {
   firstNode: Node;
   lastNode: Node;
 }
 
-export default class Component<Args extends {} = {}> extends BaseComponent<Args> {
-  args: Args;
+export default class Component<S = unknown> extends BaseComponent<S> {
+  args: Readonly<Args<S>>;
 
   /**
    * Development-mode only name of the component, useful for debugging.

packages/@glimmer/component/test/browser/args-test.ts

@@ -6,7 +6,7 @@ const { module, test } = QUnit;
 
 module('[@glimmer/component] Component Arguments');
 
-test('Getters that depend on `args` re-render correctly', async function(assert) {
+test('Getters that depend on `args` re-render correctly', async function (assert) {
   assert.expect(2);
 
   let parent: ParentComponent;
@@ -22,11 +22,7 @@ test('Getters that depend on `args` re-render correctly', async function(assert)
     }
   }
 
-  class ChildComponent extends Component {
-    args: {
-      firstName: string
-    };
-
+  class ChildComponent extends Component<{ Args: { firstName: string } }> {
     get name() {
       return `${this.args.firstName} Dale`;
     }
@@ -46,7 +42,7 @@ test('Getters that depend on `args` re-render correctly', async function(assert)
     .template('ChildComponent', '<div>{{name}} {{@status}}</div>')
     .boot();
 
-  setPropertyDidChange(function() {
+  setPropertyDidChange(function () {
     app.scheduleRerender();
   });