Skip to content
This repository has been archived by the owner on Oct 29, 2024. It is now read-only.

Component signature #385

Merged
merged 7 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"test:babel-plugins": "yarn workspace @glimmer/babel-preset test",
"test:browsers": "testem ci",
"test:ember": "yarn workspace @glimmer/component ember try:one",
"test:types": "dtslint test/types",
"test:types": "tsc --noEmit --project test/types && dtslint test/types",
"test:watch": "testem"
},
"browserslist": {
Expand Down Expand Up @@ -53,6 +53,7 @@
"@typescript-eslint/parser": "^4.18.0",
"babel-loader": "^8.1.0",
"dtslint": "^3.4.1",
"expect-type": "~0.13.0",
"eslint": "^6.8.0",
"eslint-config-prettier": "^6.10.1",
"eslint-plugin-prettier": "^3.1.2",
Expand Down
64 changes: 59 additions & 5 deletions packages/@glimmer/component/addon/-private/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,66 @@ export function setDestroyed(component: GlimmerComponent<object>): void {
DESTROYED.set(component, true);
}

export let ARGS_SET: WeakMap<object, 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 --- //

// 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.
declare const Empty: unique symbol;
type EmptyObject = { [Empty]?: true };

type GetOrElse<Obj, K, Fallback> = K extends keyof Obj ? Obj[K] : Fallback;

/** Given a signature `S`, get back the `Args` type. */
type ArgsFor<S> = 'Args' extends keyof S
? S['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 */
type ExpandSignature<T> = {
chriskrycho marked this conversation as resolved.
Show resolved Hide resolved
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: 'Blocks' extends keyof T
? {
[Block in keyof T['Blocks']]: T['Blocks'][Block] extends unknown[]
? { Positional: T['Blocks'][Block] }
: T['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
Expand Down Expand Up @@ -139,7 +193,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 GlimmerComponent<Args extends {} = {}> {
export default class GlimmerComponent<S = unknown> {
/**
* Constructs a new component and assigns itself the passed properties. You
* should not construct new components yourself. Instead, Glimmer will
Expand All @@ -148,7 +202,7 @@ export default class GlimmerComponent<Args extends {} = {}> {
* @param owner
* @param args
*/
constructor(_owner: unknown, args: Args) {
constructor(_owner: unknown, args: Args<S>) {
if (DEBUG && !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.`
Expand Down Expand Up @@ -185,7 +239,7 @@ export default class GlimmerComponent<Args extends {} = {}> {
* <p>Welcome, {{@firstName}} {{@lastName}}!</p>
* ```
*/
args: Readonly<Args>;
readonly args: Readonly<Args<S>>;

get isDestroying(): boolean {
return DESTROYING.get(this) || false;
Expand Down
6 changes: 3 additions & 3 deletions packages/@glimmer/component/addon/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ declare module '@ember/component' {
import { setComponentManager } from '@ember/component';

import GlimmerComponentManager from './-private/ember-component-manager';
import _GlimmerComponent from './-private/component';
import _GlimmerComponent, { Args } from './-private/component';
import { setOwner } from '@ember/application';

export default class GlimmerComponent extends _GlimmerComponent {
constructor(owner, args) {
export default class GlimmerComponent<S> extends _GlimmerComponent<S> {
chriskrycho marked this conversation as resolved.
Show resolved Hide resolved
constructor(owner: object, args: Args<S>) {
chriskrycho marked this conversation as resolved.
Show resolved Hide resolved
super(owner, args);

if (DEBUG && !(owner !== null && typeof owner === 'object')) {
Expand Down
6 changes: 3 additions & 3 deletions packages/@glimmer/component/src/component.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { setComponentManager, setOwner } from '@glimmer/core';
import GlimmerComponentManager from './component-manager';
import _GlimmerComponent from '../addon/-private/component';
import _GlimmerComponent, { Args } from '../addon/-private/component';
import { DEBUG } from '@glimmer/env';

export default class GlimmerComponent<Args extends {} = {}> extends _GlimmerComponent<Args> {
constructor(owner: object, args: Args) {
export default class GlimmerComponent<S> extends _GlimmerComponent<S> {
chriskrycho marked this conversation as resolved.
Show resolved Hide resolved
constructor(owner: object, args: Args<S>) {
super(owner, args);

if (DEBUG && !(owner !== null && typeof owner === 'object')) {
Expand Down
4 changes: 3 additions & 1 deletion packages/@glimmer/tracking/src/tracked.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,9 @@ function descriptorForField<T extends object, K extends keyof T>(
): PropertyDescriptor {
if (DEBUG && desc && (desc.value || desc.get || desc.set)) {
throw new Error(
`You attempted to use @tracked on ${key}, but that element is not a class field. @tracked is only usable on class fields. Native getters and setters will autotrack add any tracked fields they encounter, so there is no need mark getters and setters with @tracked.`
`You attempted to use @tracked on ${String(
key
)}, but that element is not a class field. @tracked is only usable on class fields. Native getters and setters will autotrack add any tracked fields they encounter, so there is no need mark getters and setters with @tracked.`
);
}

Expand Down
105 changes: 81 additions & 24 deletions test/types/component-test.ts
Original file line number Diff line number Diff line change
@@ -1,39 +1,96 @@
import { expectTypeOf } from 'expect-type';
import * as gc from '@glimmer/component';
import { hasExactKeys } from './utils';

const Component = gc.default;

hasExactKeys<{
default: unknown;
}>()(gc);

// $ExpectType typeof GlimmerComponent
gc.default;
expectTypeOf(gc).toHaveProperty('default');
expectTypeOf(gc.default).toEqualTypeOf<typeof Component>();

type Args = {
foo: number;
};

const component = new Component<Args>({}, { foo: 123 });
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>();

interface ArgsOnly {
Args: Args;
}

const componentWithArgsOnly = new Component<ArgsOnly>({}, { foo: 123 });
expectTypeOf(componentWithArgsOnly.args).toEqualTypeOf<Readonly<Args>>();

interface ElementOnly {
Element: HTMLParagraphElement;
}

const componentWithElOnly = new Component<ElementOnly>({}, {});

// We cannot check on toEqualTypeOf here b/c EmptyObject is intentionally not
// public.
chriskrycho marked this conversation as resolved.
Show resolved Hide resolved
expectTypeOf(componentWithElOnly.args).toMatchTypeOf<Readonly<{}>>();

interface Blocks {
default: [name: string];
inverse: [];
}

interface BlockOnlySig {
Blocks: Blocks;
}

const componentWithBlockOnly = new Component<BlockOnlySig>({}, {});

// We cannot check on toEqualTypeOf here b/c EmptyObject is intentionally not
// public.
expectTypeOf(componentWithBlockOnly.args).toMatchTypeOf<Readonly<{}>>();

interface ArgsAndBlocks {
Args: Args;
Blocks: Blocks;
}

const componentwithArgsAndBlocks = new Component<ArgsAndBlocks>({}, { foo: 123 });
expectTypeOf(componentwithArgsAndBlocks.args).toEqualTypeOf<Readonly<Args>>();

hasExactKeys<{
args: unknown;
isDestroying: unknown;
isDestroyed: unknown;
willDestroy: unknown;
}>()(component);
interface ArgsAndEl {
Args: Args;
Element: HTMLParagraphElement;
}

// $ExpectType Readonly<Args>
component.args;
const componentwithArgsAndEl = new Component<ArgsAndEl>({}, { foo: 123 });
expectTypeOf(componentwithArgsAndEl.args).toEqualTypeOf<Readonly<Args>>();

// $ExpectType boolean
component.isDestroying;
interface FullShortSig {
Args: Args;
Element: HTMLParagraphElement;
Blocks: Blocks;
}

// $ExpectType boolean
component.isDestroyed;
const componentWithFullShortSig = new Component<FullShortSig>({}, { foo: 123 });
expectTypeOf(componentWithFullShortSig.args).toEqualTypeOf<Readonly<Args>>();

// $ExpectType () => void
component.willDestroy;
interface FullLongSig {
Args: {
Named: Args;
Positional: [];
Copy link
Member

Choose a reason for hiding this comment

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

😢

};
Element: HTMLParagraphElement;
Blocks: {
default: {
Params: {
Positional: [name: string];
};
};
};
}

// $ExpectError
component.args.bar = 123;
const componentWithFullSig = new Component<FullLongSig>({}, { foo: 123 });
expectTypeOf(componentWithFullSig.args).toEqualTypeOf<Readonly<Args>>();
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6898,6 +6898,11 @@ expand-tilde@^2.0.0, expand-tilde@^2.0.2:
dependencies:
homedir-polyfill "^1.0.1"

expect-type@~0.13.0:
version "0.13.0"
resolved "https://registry.yarnpkg.com/expect-type/-/expect-type-0.13.0.tgz#916646a7a73f3ee77039a634ee9035efe1876eb2"
integrity sha512-CclevazQfrqo8EvbLPmP7osnb1SZXkw47XPPvUUpeMz4HuGzDltE7CaIt3RLyT9UQrwVK/LDn+KVcC0hcgjgDg==

express@^4.10.7, express@^4.17.1:
version "4.17.2"
resolved "https://registry.npmjs.org/express/-/express-4.17.2.tgz#c18369f265297319beed4e5558753cc8c1364cb3"
Expand Down