Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Component signature #385

Merged
merged 7 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
85 changes: 80 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,87 @@ 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 --- //
// 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, 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.
*
* @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: '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 +214,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 +223,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 +260,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 = unknown> extends _GlimmerComponent<S> {
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 = unknown> extends _GlimmerComponent<S> {
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
107 changes: 83 additions & 24 deletions test/types/component-test.ts
Original file line number Diff line number Diff line change
@@ -1,39 +1,98 @@
import { expectTypeOf } from 'expect-type';
import * as gc from '@glimmer/component';
import { hasExactKeys } from './utils';

const Component = gc.default;
// 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*
// this import location, we've broken any existing declarations published using
// the current type signatures.
import { EmptyObject } from '@glimmer/component/addon/-private/component';

hasExactKeys<{
default: unknown;
}>()(gc);
const Component = gc.default;

// $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>({}, {});

expectTypeOf(componentWithElOnly.args).toEqualTypeOf<Readonly<EmptyObject>>();

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

interface BlockOnlySig {
Blocks: Blocks;
}

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

expectTypeOf(componentWithBlockOnly.args).toEqualTypeOf<Readonly<EmptyObject>>();

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