Skip to content

Commit

Permalink
[BUGFIX] improve component argument types for TS-JS interop
Browse files Browse the repository at this point in the history
`Component` currently types its arguments as extending `{}`:

    class Component<Args extends {} = {}> {
      readonly args: Args;
      constructor(owner: unknown, args: Args);
    }

This works in pure-JS or pure-TS codebases, but breaks down in mixed
codebases, especially where users are using the `@ts-check` pragma or
where we want to feed information from component definitions into the TS
language server to get better completion info throughout an app.

For example, given this component in a JS codebase with `@ts-check`
enabled:

    class Profile extends Component {
      get description() {
        return `${this.args.name} is ${this.args.age} years old!`;
      }
    }

The TS language server (e.g. in an editor) will report:

> Property 'name' does not exist on type '{}'.
> Property 'age' does not exist on type '{}'.

To eliminate this problem while maintaining compatibility with all
existing typed usages (including some we might like to forbid but cannot
at present, like passing `Component<[]>` -- thanks JavaScript!), we
switch from `{}` to `Record<string, any>`:

    class Component<Args extends Record<string, any> = Record<string, any>> {
      readonly args: Args;
      constructor(owner: unknown, args: Args);
    }

Now, the error described above is eliminated for JS consumers using TS
information in any way, but TS users can continue to supply correct
types for their components.
  • Loading branch information
chriskrycho committed Apr 24, 2020
1 parent 29e4c47 commit 9923764
Showing 1 changed file with 13 additions and 1 deletion.
14 changes: 13 additions & 1 deletion packages/@glimmer/component/addon/-private/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,18 @@ if (DEBUG) {
ARGS_SET = new WeakMap();
}

// We use `Record<string, any>` here to support rigorous typing in TS, clean
// (effectively invisible) usage in JS, and clean interop in mixed code bases.
// It needs to be a dictionary-like object with string keys, but `{}` will not
// work because in a context where the args are not set, but information about
// the component is passed to a TS aware context, it will throw (e.g. for users
// with `// @check-js` pragmas, or in a language server built on top of TS LS).
// At the same time, the type must be compatible with narrowing to more specific
// arguments, which rules out `Record<string, unknown>`, forcing us to disable
// `no-explicit-any` here.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
type MinimalArgs = Record<string, any>;

/**
* 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 +151,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<Args extends MinimalArgs = MinimalArgs> {
/**
* Constructs a new component and assigns itself the passed properties. You
* should not construct new components yourself. Instead, Glimmer will
Expand Down

0 comments on commit 9923764

Please sign in to comment.