Support Signature types for modifiers

Details
-------

Following the design pioneered in emberjs/rfcs#748, introduce a new
`Signature` type and use it in defining the types for both function-
and class-based modifiers. This allows end users to write, for example:

    interface PlaySig {
      Args: {
        Named: {
          when: boolean;
        }
      }
      Element: HTMLMediaElement;
    }

    export default modifier<PlaySig>((el, _, { when: shouldPlay }) => {
      if (shouldPlay) {
        el.play();
      } else {
        el.pause();
      }
    });

This supports both `Named` and `Positional` args, and works equally
well with class-based modifiers. It also provides the `Element` type as
`Element` by default, and provides internal type utilities as well as
one external-facing type utility to make writing the type of `args`
when passed to a modifier `constructor` easy to do in terms of the
`Signature` which represents the modifier.

Users do not *need* to type this form: the form using basic inference
continues to work as well:

    export default modifier((
      el: HTMLMediaElement,
      _: [],
      { when: shouldPlay }: { when: boolean }
    ) => {
      if (shouldPlay) {
        el.play();
      } else {
        el.pause();
      }
    }));

That is: the support for `Signatures` is strictly *new* capability
which was not present before.

The same kind of signature works with class-based modifiers as well,
while retaining backward compatibility with the `Args`-based signature
and declaration on the subclass `element` field directly. To support
that in a way that does not require users to name their args over and
over again, introduce an `ArgsFor` type utility which translates from the signature type to the runtime form:

    import Modifier, { ArgsFor } from 'ember-modifier';

    interface PlaySig {
      Args: {
        Named: {
          when: boolean;
        };
      };
      Element: HTMLMediaElement;
    }

    class MyModifier extends Modifier<PlaySig> {
      constructor(owner: unknown, args: ArgsFor<PlaySig>) {
        super(owner, args);
        // ...
      }

      didReceiveArguments() {
        const shouldPlay = this.args.named.when;
        if (shouldPlay) {
          this.element.play();
        } else {
          this.element.pause();
        }
      }
    }

Additionally, the `modifier` function now returns an opaque
`FunctionBasedModifier` type. This mostly exists to provide nice hooks
for tooling to hook onto, but it has the added benefit of showing
something besides `unknown` in an editor, and that "something" shows
the arguments and element specified for the modifier.

Supporting changes
------------------

-   Update the README to match, and fix a *lot* of long-since outdated
    documentation along the way.

-   Loosen type test constraint to support older TS versions

    The new type signature *is* compatible (as the tests on TS 4.5+
    show), but on older versions of TS, it does not check under a type
    equality check, only under a type matching check. For the purposes
    of the specific test in question, that's perfectly fine, because
    the *other* type tests cover inference and resolution correctly.

README.md

@@ -697,7 +697,17 @@ Both the functional and class APIs can be used with TypeScript!
 
 Before checking out the [Examples with Typescript](#examples-with-type-script) below, there is an important caveat you should understand about type safety!
 
-True type safety requires runtime checking, since templates are not currently type-checked: the arguments passed to your modifier can be *anything*. They’re typed as `unknown` by default, which means by default TypeScript will *require* you to work out the type passed to you at runtime. For example, with the `ScrollPositionModifier` shown above, you can combine TypeScript’s [type narrowing] with the default types for the class to provide runtime errors if the caller passes the wrong types, while providing safety throughout the rest of the body of the modifier. Here, `didReceiveArguments` would be *guaranteed* to have the correct types for `this.scrollPosition` and `this.isRelative`:
+There are, today, two basic approaches you can take to dealing with your modifier's arguments and element in a type safe way:
+
+1. You can use a type definition which specifies those for the outside world, relying on tooling like [Glint][glint] to check that the invocation is correct, and treat input as safe accordingly.
+2. You can provide the minimal public interface which *all* modifiers conform to, and do runtime type checking with `assert` calls to make your internal implementation safe.
+
+If you have a code base which is strictly typed from end to end, including with template type checking via Glint, then (1) is a great choice. If you have a mixed code base, or are publishing an addon for others to use, then [it's usually best to do both (1) *and* (2)][safe-ts-libs]!
+
+[glint]: https://github.com/typed-ember/glint
+[safe-ts-libs]: https://v5.chriskrycho.com/journal/writing-robust-typescript-libraries/s
+
+To handle runtime checking, for non-type-checked templates (including projects not yet using Glint or supporting external callers), you should *act* as though the arguments passed to your modifier can be *anything*. They’re typed as `unknown` by default, which means by default TypeScript will *require* you to work out the type passed to you at runtime. For example, with the `ScrollPositionModifier` shown above, you can combine TypeScript’s [type narrowing] with the default types for the class to provide runtime errors if the caller passes the wrong types, while providing safety throughout the rest of the body of the modifier. Here, `didReceiveArguments` would be *guaranteed* to have the correct types for `this.scrollPosition` and `this.isRelative`:
 
 [type narrowing]: https://www.typescriptlang.org/docs/handbook/advanced-types.html#type-guards-and-differentiating-types
 
@@ -736,30 +746,82 @@ export class ScrollPositionModifier extends ClassBasedModifier {
 }
 ```
 
-You can also avoid writing these runtime checks by extending `Modifier` with predefined args, similar to the way you would define your args for a Glimmer Component:
+If you were writing for a fully-typed context, you can define your `Modifier` with a `Signature` interface, similar to the way you would define your signature for a Glimmer Component:
 
 ```ts
 // app/modifiers/scroll-position.ts
 import Modifier from 'ember-modifier';
 
-interface ScrollPositionModifierArgs {
-  positional: [number],
-  named: {
-    relative: boolean
-  }
+interface ScrollPositionModifierSignature {
+  Args: {
+    Positional: [number];
+    Named: {
+      relative: boolean;
+    };
+  };
+  Element: Element; // not required: it'll be set by default
 }
 
-export default class ScrollPositionModifier extends Modifier<ScrollPositionModifierArgs> {
+export default class ScrollPositionModifier
+    extends Modifier<ScrollPositionModifierSignature> {
   get scrollPosition(): number {
     return this.args.positional[0];
   }
 
   get isRelative(): boolean {
-    return this.args.named.relative
+    return this.args.named.relative;
   }
 
   didReceiveArguments() {
-    if(this.isRelative) {
+    if (this.isRelative) {
+      this.element.scrollTop += this.scrollPosition;
+    } else {
+      this.element.scrollTop = this.scrollPosition;
+    }
+  }
+}
+```
+
+Besides supporting integration with [Glint][glint], this also provides nice hooks for documentation tooling. Note, however, that it can result in *much worse* feedback in tests or at runtime if someone passes the wrong kind of arguments to your modifier and you *haven't* included the assertions: users who pass the wrong thing will just have the modifier fail. For example, if you fail to pass the positional argument, `this.scrollPosition` would simply be `undefined`, and then `this.element.scrollTop` could end up being set to `NaN`. Whoops! For that reason, if your modifier will be used by non-TypeScript consumers, you should both publish the types for it *and* add dev-time assertions:
+
+```ts
+// app/modifiers/scroll-position.ts
+import Modifier from 'ember-modifier';
+
+interface ScrollPositionModifierSignature {
+  Args: {
+    Positional: [number];
+    Named: {
+      relative: boolean;
+    };
+  };
+  Element: Element; // not required: it'll be set by default
+}
+
+export default class ScrollPositionModifier
+    extends Modifier<ScrollPositionModifierSignature> {
+  get scrollPosition(): number {
+    const scrollValue = this.args.positional[0];
+    assert(,
+      `first argument to 'scroll-position' must be a number, but ${scrollValue} was ${typeof scrollValue}`,
+      typeof scrollValue === "number"
+    );
+
+    return scrollValue;
+  }
+
+  get isRelative(): boolean {
+    const { relative } = this.args.named;
+    assert(
+      `'relative' argument to 'scroll-position' must be a boolean, but ${relative} was ${typeof relative}`,
+      typeof relative === "boolean"
+    );
+
+    return relative;
+  }
+
+  didReceiveArguments() {
+    if (this.isRelative) {
       this.element.scrollTop += this.scrollPosition;
     } else {
       this.element.scrollTop = this.scrollPosition;
@@ -768,13 +830,71 @@ export default class ScrollPositionModifier extends Modifier<ScrollPositionModif
 }
 ```
 
-However, while doing so is slightly more convenient, it means you get *much worse* feedback in tests or at runtime if someone passes the wrong kind of arguments to your modifier.
+### The `Signature` type
+
+The `Signature` for a modifier is the combination of the positional and named arguments it receives and the element to which it may be applied.
+
+```ts
+interface Signature {
+  Args: {
+    Named: {
+      [argName: string]: unknown;
+    };
+    Positional: unknown[];
+  };
+  Element: Element;
+}
+```
+
+When writing a signature yourself, all of those are optional: the types for modifiers will fall back to the correct defaults of `Element`, an object for named arguments, and an array for positional arguments. You can apply a signature when defining either a function-based or a class-based modifier.
+
+In a function-based modifier, the callback arguments will be inferred from the signature, so you do not need to specify the types twice:
+
+```ts
+interface MySignature {
+  // ...
+}
+
+const myModifier = modifier<MySignature>((el, pos, named) => {
+  // ...
+})
+```
+
+You never *need* to specify a signature in this way for a function-based modifier. However, it is tested to keep working, since it can be useful for documentation!
+
+The same basic approach works with a class-based modifier:
+
+```ts
+interface MySignature {
+  // ...
+}
+
+export default class MyModifier extends Modifier<MySignature> {
+  // ...
+}
+```
+
+In that case, the `element` and `args` will always have the right types throughout the body. Since the type of `args` in the constructor are derived from the signature, you can use the `ArgsFor` type helper to avoid having to write the type out separately:
+
+```ts
+import Modifier, { ArgsFor } from 'ember-modifier';
+
+interface MySignature {
+  // ...
+}
+
+export default class MyModifier extends Modifier<MySignature> {
+  constructor(owner: unknown, args: ArgsFor<MySignature>) {
+    // ...
+  }
+}
+```
 
 ### Examples with TypeScript
 
-#### Functional modifier
+#### Function-based modifier
 
-Let’s look at a variant of the `move-randomly` example from above, implemented in TypeScript, and now requiring a named argument, the maximum offset. Using the recommended runtime type-checking, it would look like this:
+Let’s look at a variant of the `move-randomly` example from above, implemented in TypeScript, and now requiring a named argument, the maximum offset. Using the recommended combination of types and runtime type-checking, it would look like this:
 
 ```ts
 // app/modifiers/move-randomly.js
@@ -783,7 +903,7 @@ import { assert } from '@ember/debug';
 
 const { random, round } = Math;
 
-export default modifier((element, _, named) => {
+export default modifier((element: HTMLElement, _: [], named: { maxOffset: number }) => {
   assert(
     'move-randomly can only be installed on HTML elements!',
     element instanceof HTMLElement
@@ -807,7 +927,7 @@ export default modifier((element, _, named) => {
 
 A few things to notice here:
 
-1.  TypeScript correctly infers the types of the arguments for the function passed to the modifier; you don't need to specify what `element` or `positional` or `named` are.
+1.  TypeScript correctly infers the *base* types of the arguments for the function passed to the modifier; you don't need to specify what `element` or `positional` or `named` are unless you are doing like we are in this example and providing a usefully more-specific type to callers.
 
 2.  If we returned a teardown function which had the wrong type signature, that would also be an error.
 
@@ -824,10 +944,7 @@ A few things to notice here:
     TypeScript will report:
 
     > ```
-    > Argument of type '(element: Element, _: Positional, named: Record<string, unknown>) => Timeout' is not assignable to parameter of type 'FunctionalModifier<Positional, Record<string, unknown>>'.
-    >   Type 'Timeout' is not assignable to type 'void | Teardown'.
-    >     Type 'Timeout' is not assignable to type 'Teardown'.
-    >       Type 'Timeout' provides no match for the signature '(): void'.
+    > Type 'Timeout' is not assignable to type 'void | Teardown'.
     > ```
 
     Likewise, if we return a function with the wrong signature, we will see the same kinds of errors. If we expected to receive an argument in the teardown callback, like this:
@@ -843,23 +960,30 @@ A few things to notice here:
     TypeScript will report:
 
     > ```
-    > Argument of type '(element: Element, _: Positional, named: Record<string, unknown>) => (interval: number) => void' is not assignable to parameter of type 'FunctionalModifier<Positional, Record<string, unknown>>'.
-    >   Type '(interval: number) => void' is not assignable to type 'void | Teardown'.
-    >     Type '(interval: number) => void' is not assignable to type 'Teardown'.
+    > Type '(interval: number) => void' is not assignable to type 'void | Teardown'.
     > ```
 
 ####  Class-based
 
-To support correctly typing `args` in the `constructor` for the case where you do runtime type checking, we supply a `ModifierArgs` interface import. Here’s what a fully typed modifier that alerts "This is a typesafe modifier!" an amount of time after receiving arguments that depends on the length of the first argument and an *optional* multiplier (a nonsensical thing to do, but one that illustrates a fully type-safe class-based modifier):
+To support correctly typing `args` in the `constructor` for the case where you do runtime type checking, we supply an `ArgsFor` type utility. (This is useful because the `Signature` type, matching Glimmer Component and other "invokable" items in Ember/Glimmer, has capital letters for the names of the types, while `args.named` and `args.positional` are lower-case.) Here’s how that would look with a fully typed modifier that alerts "This is a typesafe modifier!" an amount of time after receiving arguments that depends on the length of the first argument and an *optional* multiplier (a nonsensical thing to do, but one that illustrates a fully type-safe class-based modifier):
 
 ```ts
-import Modifier, { ModifierArgs } from 'ember-modifier';
+import Modifier, { ArgsFor } from 'ember-modifier';
 import { assert } from '@ember/debug';
 
-export default class NeatModifier extends Modifier {
+interface NeatSignature {
+  Args: {
+    Named: {
+      multiplier?: number;
+    };
+    Positional: [string];
+  }
+}
+
+export default class Neat extends Modifier<NeatSignature> {
   interval?: number;
 
-  constructor(owner: unknown, args: ModifierArgs) {
+  constructor(owner: unknown, args: ArgsFor<NeatSignature>) {
     super(owner, args);
     // other setup you might do
   }

addon/-private/class/modifier-manager.ts

@@ -4,14 +4,14 @@ import { set } from '@ember/object';
 import { destroy, registerDestructor } from '@ember/destroyable';
 
 import ClassBasedModifier from './modifier';
-import { ModifierArgs } from 'ember-modifier/-private/interfaces';
+import { ArgsFor, ElementFor } from 'ember-modifier/-private/signature';
 import { consumeArgs, Factory, isFactory } from '../compat';
 
-function destroyModifier(modifier: ClassBasedModifier): void {
+function destroyModifier<S>(modifier: ClassBasedModifier<S>): void {
   modifier.willDestroy();
 }
 
-export default class ClassBasedModifierManager {
+export default class ClassBasedModifierManager<S> {
   capabilities = capabilities(gte('3.22.0') ? '3.22' : '3.13');
 
   constructor(private owner: unknown) {}
@@ -20,8 +20,8 @@ export default class ClassBasedModifierManager {
     factoryOrClass:
       | Factory<typeof ClassBasedModifier>
       | typeof ClassBasedModifier,
-    args: ModifierArgs
-  ): ClassBasedModifier {
+    args: ArgsFor<S>
+  ): ClassBasedModifier<S> {
     const Modifier = isFactory(factoryOrClass)
       ? factoryOrClass.class
       : factoryOrClass;
@@ -34,9 +34,9 @@ export default class ClassBasedModifierManager {
   }
 
   installModifier(
-    instance: ClassBasedModifier,
-    element: Element,
-    args: ModifierArgs
+    instance: ClassBasedModifier<S>,
+    element: ElementFor<S>,
+    args: ArgsFor<S>
   ): void {
     instance.element = element;
 
@@ -48,7 +48,7 @@ export default class ClassBasedModifierManager {
     instance.didInstall();
   }
 
-  updateModifier(instance: ClassBasedModifier, args: ModifierArgs): void {
+  updateModifier(instance: ClassBasedModifier<S>, args: ArgsFor<S>): void {
     // TODO: this should be an args proxy
     set(instance, 'args', args);
 

addon/-private/class/modifier.ts

@@ -1,8 +1,8 @@
 import { setOwner } from '@ember/application';
 import { setModifierManager } from '@ember/modifier';
 import Manager from './modifier-manager';
-import { ModifierArgs } from 'ember-modifier/-private/interfaces';
 import { isDestroying, isDestroyed } from '@ember/destroyable';
+import { ElementFor, ArgsFor, DefaultSignature } from '../signature';
 
 /**
  * A base class for modifiers which need more capabilities than function-based
@@ -18,15 +18,13 @@ import { isDestroying, isDestroyed } from '@ember/destroyable';
  * values they access will be added to the modifier, and the modifier will
  * update if any of those values change.
  */
-export default class ClassBasedModifier<
-  Args extends ModifierArgs = ModifierArgs
-> {
+export default class ClassBasedModifier<S = DefaultSignature> {
   /**
    * The arguments passed to the modifier. `args.positional` is an array of
    * positional arguments, and `args.named` is an object containing the named
    * arguments.
    */
-  readonly args: Args;
+  readonly args: ArgsFor<S>;
 
   /**
    * The element the modifier is applied to.
@@ -37,9 +35,9 @@ export default class ClassBasedModifier<
   // SAFETY: this is managed correctly by the class-based modifier. It is not
   // available during the `constructor`.
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  element: Element = null as any;
+  element: ElementFor<S> = null as any;
 
-  constructor(owner: unknown, args: Args) {
+  constructor(owner: unknown, args: ArgsFor<S>) {
     setOwner(this, owner);
     this.args = args;
   }

addon/-private/compat.ts

@@ -1,5 +1,5 @@
-import { ModifierArgs } from './interfaces';
 import { gte } from 'ember-compatibility-helpers';
+import { ArgsFor } from './signature';
 
 export interface Factory<T> {
   owner: unknown;
@@ -27,10 +27,10 @@ const noop = (): void => {};
  * avoid introducing a breaking change until a suitable transition path is made
  * available.
  */
-let consumeArgs: (args: ModifierArgs) => void = noop;
+let consumeArgs: (args: ArgsFor<any>) => void = noop;
 
 if (gte('3.22.0')) {
-  consumeArgs = function ({ positional, named }) {
+  consumeArgs = function ({ positional, named }: ArgsFor<any>) {
     for (let i = 0; i < positional.length; i++) {
       positional[i];
     }