Merge pull request #390 from glimmerjs/bugfix-signature

Fix Signature handling for unions and generics

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

@@ -54,18 +54,7 @@ type ArgsFor<S> = S extends { Args: infer Args }
     : { 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> = {
+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
@@ -74,11 +63,29 @@ export type ExpandSignature<T> = {
     ? {
         [Block in keyof Blocks]: Blocks[Block] extends unknown[]
           ? { Positional: Blocks[Block] }
-          : Blocks[Block];
+          : Blocks[Block]
       }
     : EmptyObject;
 };
 
+/**
+ * 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***.
+ */
+// The conditional type here is because TS applies conditional types
+// distributively. This means that for union types, checks like `keyof T` get
+// all the keys from all elements of the union, instead of ending up as `never`
+// and then always falling into the `Signature` path instead of falling back to
+// the legacy args handling path.
+export type ExpandSignature<T> = T extends any ? _ExpandSignature<T> : never;
+
 /**
  * @internal we use this type for convenience internally; inference means users
  *   should not normally need to name it
@@ -220,7 +227,9 @@ export default class BaseComponent<S = unknown> {
   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.`
       );
     }
 

test/types/component-test.ts

@@ -1,5 +1,6 @@
 import { expectTypeOf } from 'expect-type';
 import * as gc from '@glimmer/component';
+import Component from '@glimmer/component';
 
 // Imported from non-public-API so we can check that we are publishing what we
 // expect to be -- and this keeps us honest about the fact that if we *change*
@@ -13,31 +14,58 @@ import * as gc from '@glimmer/component';
 // which they should not use in any way, this is "safe" from a public API POV.
 import { EmptyObject } from '@glimmer/component/dist/types/addon/-private/component';
 
-const Component = gc.default;
+declare let basicComponent: Component;
+expectTypeOf(basicComponent).toHaveProperty('args');
+expectTypeOf(basicComponent).toHaveProperty('isDestroying');
+expectTypeOf(basicComponent).toHaveProperty('isDestroyed');
+expectTypeOf(basicComponent).toHaveProperty('willDestroy');
+expectTypeOf(basicComponent.isDestroying).toEqualTypeOf<boolean>();
+expectTypeOf(basicComponent.isDestroyed).toEqualTypeOf<boolean>();
+expectTypeOf(basicComponent.willDestroy).toEqualTypeOf<() => void>();
 
 expectTypeOf(gc).toHaveProperty('default');
 expectTypeOf(gc.default).toEqualTypeOf<typeof Component>();
 
-type Args = {
+type LegacyArgs = {
   foo: number;
 };
 
-const componentWithLegacyArgs = new Component<Args>({}, { foo: 123 });
-expectTypeOf(componentWithLegacyArgs).toHaveProperty('args');
-expectTypeOf(componentWithLegacyArgs).toHaveProperty('isDestroying');
-expectTypeOf(componentWithLegacyArgs).toHaveProperty('isDestroyed');
-expectTypeOf(componentWithLegacyArgs).toHaveProperty('willDestroy');
-expectTypeOf(componentWithLegacyArgs.args).toEqualTypeOf<Readonly<Args>>();
-expectTypeOf(componentWithLegacyArgs.isDestroying).toEqualTypeOf<boolean>();
-expectTypeOf(componentWithLegacyArgs.isDestroyed).toEqualTypeOf<boolean>();
-expectTypeOf(componentWithLegacyArgs.willDestroy).toEqualTypeOf<() => void>();
+const componentWithLegacyArgs = new Component<LegacyArgs>({}, { foo: 123 });
+expectTypeOf(componentWithLegacyArgs.args).toEqualTypeOf<Readonly<LegacyArgs>>();
+
+// Here, we are testing that the types propertly distribute over union types,
+// generics which extend other types, etc.
+type LegacyArgsDistributive = { foo: number } | { bar: string; baz: boolean };
+
+const legacyArgsDistributiveA = new Component<LegacyArgsDistributive>({}, { foo: 123 });
+expectTypeOf(legacyArgsDistributiveA.args).toEqualTypeOf<Readonly<LegacyArgsDistributive>>();
+const legacyArgsDistributiveB = new Component<LegacyArgsDistributive>({}, { bar: "hello", baz: true });
+expectTypeOf(legacyArgsDistributiveB.args).toEqualTypeOf<Readonly<LegacyArgsDistributive>>();
+
+interface ExtensibleLegacy<T> {
+  value: T;
+  extras: boolean;
+  funThings: string[];
+}
+
+class WithExtensibleLegacy<T extends ExtensibleLegacy<unknown>> extends Component<T> {}
+declare const withExtensibleLegacy: WithExtensibleLegacy<ExtensibleLegacy<unknown>>;
+expectTypeOf(withExtensibleLegacy.args.value).toEqualTypeOf<unknown>();
+expectTypeOf(withExtensibleLegacy.args.extras).toEqualTypeOf<boolean>();
+expectTypeOf(withExtensibleLegacy.args.funThings).toEqualTypeOf<string[]>();
+
+interface Extended extends ExtensibleLegacy<string> {}
+
+class WithExtensibleLegacySubclass extends WithExtensibleLegacy<Extended> {}
+declare const withExtensibleLegacySubclass: WithExtensibleLegacySubclass;
+expectTypeOf(withExtensibleLegacySubclass.args.value).toEqualTypeOf<string>();
 
 interface ArgsOnly {
-  Args: Args;
+  Args: LegacyArgs;
 }
 
 const componentWithArgsOnly = new Component<ArgsOnly>({}, { foo: 123 });
-expectTypeOf(componentWithArgsOnly.args).toEqualTypeOf<Readonly<Args>>();
+expectTypeOf(componentWithArgsOnly.args).toEqualTypeOf<Readonly<LegacyArgs>>();
 
 interface ElementOnly {
   Element: HTMLParagraphElement;
@@ -61,33 +89,33 @@ const componentWithBlockOnly = new Component<BlockOnlySig>({}, {});
 expectTypeOf(componentWithBlockOnly.args).toEqualTypeOf<Readonly<EmptyObject>>();
 
 interface ArgsAndBlocks {
-  Args: Args;
+  Args: LegacyArgs;
   Blocks: Blocks;
 }
 
 const componentwithArgsAndBlocks = new Component<ArgsAndBlocks>({}, { foo: 123 });
-expectTypeOf(componentwithArgsAndBlocks.args).toEqualTypeOf<Readonly<Args>>();
+expectTypeOf(componentwithArgsAndBlocks.args).toEqualTypeOf<Readonly<LegacyArgs>>();
 
 interface ArgsAndEl {
-  Args: Args;
+  Args: LegacyArgs;
   Element: HTMLParagraphElement;
 }
 
 const componentwithArgsAndEl = new Component<ArgsAndEl>({}, { foo: 123 });
-expectTypeOf(componentwithArgsAndEl.args).toEqualTypeOf<Readonly<Args>>();
+expectTypeOf(componentwithArgsAndEl.args).toEqualTypeOf<Readonly<LegacyArgs>>();
 
 interface FullShortSig {
-  Args: Args;
+  Args: LegacyArgs;
   Element: HTMLParagraphElement;
   Blocks: Blocks;
 }
 
 const componentWithFullShortSig = new Component<FullShortSig>({}, { foo: 123 });
-expectTypeOf(componentWithFullShortSig.args).toEqualTypeOf<Readonly<Args>>();
+expectTypeOf(componentWithFullShortSig.args).toEqualTypeOf<Readonly<LegacyArgs>>();
 
 interface FullLongSig {
   Args: {
-    Named: Args;
+    Named: LegacyArgs;
     Positional: [];
   };
   Element: HTMLParagraphElement;
@@ -101,4 +129,4 @@ interface FullLongSig {
 }
 
 const componentWithFullSig = new Component<FullLongSig>({}, { foo: 123 });
-expectTypeOf(componentWithFullSig.args).toEqualTypeOf<Readonly<Args>>();
+expectTypeOf(componentWithFullSig.args).toEqualTypeOf<Readonly<LegacyArgs>>();