Component arguments and Typescript - what's the right approach?

I’m at the start of my journey with Ember and Typescript and running into an error when specifying argument types in a component constructor. According to what I’ve read in the ember-cli-typescript docs I should be able to do something like this:

import Component from '@glimmer/component';
​
export interface MyComponentSignature<T> {
  Args: {
    items: T[];
  };
}
​
export default class MyComponent<T> extends Component< MyComponentSignature <T>> {
  constructor(owner: unknown, args: {}) {
    super(owner, args);
​    ...
  }
}

The current ember-cli-typescript docs state:

The args are {} because a Glimmer component always receives an object containing its arguments, even if the caller didn’t pass anything: then it would just be an empty object.

However, when args are {} I get this error:

Don't use `{}` as a type. `{}` actually means "any non-nullish value".
- If you want a type meaning "any object", you probably want `Record<string, unknown>` instead.
- If you want a type meaning "any value", you probably want `unknown` instead.
- If you want a type meaning "empty object", you probably want `Record<string, never>` instead.eslint@typescript-eslint/ban-types

Updating so that args are MyComponentSignature<T>['Args']) resolves the error but I can’t find any documentation that would suggest this is necessary. Is it?

Yeah, this is just a gap/out of date spot in the ember-cli-typescript docs: I thought we had updated to account for the shift to using Signature types (we used to only support Args, not the Element or Blocks) but it seems we missed some spots! The old explanation was also addressing specifically the case where the args were unspecified, as the code there would not have type checked with more detailed args specified the old way, either, though!

It has likely not come to our attention before because it is relatively unusual to need to have a constructor and name those args there. In the relatively rare cases where that actually is necessary, one option is to pull the args into their own interface and name that in the constructor while also attaching it to the signature interface:

import Owner from '@ember/owner';
import Component from '@glimmer/component';

interface MyArgs<T> {
  items: T[]
}

interface MySig<T> {
  Args: MyArgs<T>;
}

export default class MyComponent<T> extends Component<MySig<T>> {
  constructor(owner: Owner, args: MyArgs<T> {
    // …
  }
}

If you open a PR to our docs, I will help get it landed; otherwise I will see to it that we get it knocked out this week!

—-

Bonus: All of the above being said, I would suggest making sure you need to do something in the constructor! Many times, what I see there are things like argument validation, doing this.someTrackedState = this.args.items, trying to set up element handlers like in Ember Classic, or other such anti-patterns. The only things that usually make sense to have in a constructor are things which are

  • actually tied to the instance, like setting up a timer which should last throughout the component’s lifecycle (even if its arguments change!) and will be torn down using willDestroy as well, AND
  • not something which can be derived from the args, as those should be handled via getters or other non-side-effecting functions, AND
  • not something involving DOM manipulation, as that should happen in modifiers instead
2 Likes

Thanks @chriskrycho I really appreciate the feedback. Happy to submit a PR.

In terms of whether or not I should be using a constructor, this is a stripped down version of my component:

import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { action } from '@ember/object';

import { DateTime, Interval } from 'luxon';

export interface MySignature<T> {
  Args: {
    activeStartDate: string;
  };
}

export default class MyComponent<T> extends Component<MySignature<T>> {

  @tracked activeStartDate;

  constructor(owner: unknown, args: MySignature<T>['Args']) {
    super(owner, args);

    if (this.args.activeStartDate) {
      this.activeStartDate = DateTime.fromISO(this.args.activeStartDate);
    } else {
      this.activeStartDate = DateTime.now();
    }
  }

  @action
  setActiveStartDate(date: DateTime) {
    this.activeStartDate = date;
  }
}

I’m trying to find a pattern that allows for a) @tracked activeStartDate to have default value; b) the parent to provide a default value (passing down a query param on a parent controller, for example); and, c) for the value to be set by an action (from a child component, for example.)

I’m unclear if this would be considered an anti-pattern but also can’t see how else to achieve the objective?

You can always use prop + getter to approximate an arg with default value, but in this case you can probably just do something like:

@tracked activeStartDate = this.args.activeStartDate ? DateTime.fromISO(this.args.activeStartDate) : DateTime.now();

EDIT: and i think if you were to write this ^ and look at the compiled code it will probably look pretty similar to your original code above.

1 Like

Nice one @dknutsen and, as ever, thanks! :+1:t2:

1 Like

I would actually avoid doing that particular thing, as it’s doing the same thing as doing it in the constructor does, and you end up needing to do extra “juggling” to keep state in sync over time—there’s no way to set it back to using whatever the parent value is.

One solution is to do something like this instead:

export default class Example extends Component {
  // This means we always have the data if we need it, and it doesn't
  // change over the life of the component. We could *change* that to
  // have it dynamically updated to "now" whenever the getter below is
  // invoked if that were the desired behavior instead.
  #constructedAt = DateTime.now();

  @tracked _startDate: DateTime;

  get activeStartDate() {
    const local = this._startDate;
    const fromArgs = DateTime.fromISO(this.args.activeStartDate);

    return local ?? fromArgs ?? this.#constructedAt;
  }

  @action setActiveStartDate(date: DateTime) {
    this._startDate = date;
  }

  @action finalize() {
    let value = this._startDate;
    this.value = null;
    this.args.save(value);
  }
}

That makes it so you use any local value if it’s set, any passed-in default if the local value is not set, or a default if neither is set, and saves the value only when you’re ready—and resets the local state at that point as well. Note that this never sets local tracked state to a copy of the external tracked state. That kind of “forking” of tracked state is super difficult to keep in sync as the code evolves over time. Instead, this approach just knows how to merge local and external tracked state, which can even be extracted into a single “pure” function (no mutation, just inputs → outputs) and tested directly if it’s complicated enough.

However, while this is an improvement, that last action smells to me: if I forget to null out the local value, the behavior locally will be buggy. As a result, I actually prefer to push all of this state management back up to the parent. While it can feel nice to have the local class “encapsulating” its state, in my experience any change to how this state works involves changing both components… which means that it isn’t actually the right boundary; “code that changes together should live together.” Here’s a simplified example (using <template> for convenience, and with slightly different mechanics than your code but in a way that hopefully translates pretty easily):

import Component from '@glimmer/component';
import { on } from '@ember/modifier';

const Child = <template>
  <form {{on "submit" @save}}>
    <label>
      Some data:
      <input {{on "change" @change}} value={{@value}} />
    </label>
    <button type="submit">Save</button>
  </form>
</template>;

const DEFAULT_VALUE = 'some default value';

export default class Parent extends Component {
  @tracked state;
  @tracked draftState;

  get valueToUse() {
    return this.draftState ?? this.state ?? DEFAULT_VALUE;
  }

  @action change({ target: { value } }) {
    this.draftState = value;
  }

  @action save(event) {
    event.preventDefault();

    this.state = this.draftState;
    this.draftState = null;
  }

  <template>
    <Child
      @value={{this.valueToUse}}
      @change={{this.change}}
      @save={{this.save}}
    />
  </template>
}

This still means you have to keep straight the relationship between state and draftState, but this way that relationship all lives together. If at some you point stop needing that distinction, or if at some point you need to add more complexity to how it works, that’s all in one place.

It’s very intuitive to try to put draft form state “local” to a component, but in practice it seems to just end up causing more churn, and I think it’s precisely because it isn’t actually the case that the state relationship is local to the form field (or similar) in question. You can see that even more clearly in the case where saving should fail if the data isn’t valid: in that case it’s trivial for the Parent class to just not clear out the draftState, but doing that in the case where the draft state lives in the child is… well, it’s difficult at best.

2 Likes