Reflect the introduction of Component Signatures #1530
Conversation
This PR follows [feedback](https://discuss.emberjs.com/t/component-arguments-and-typescript-whats-the-right-approach/19761) from @chriskrycho on Ember Discuss, and aims to reflect the introduction of Component Signatures. Hopefully this isn't too heavy handed but it seemed to me that previous sections belonged to a time when `args` could be empty object. As defining a Signature is a prerequisite for calling `this.args` it felt right to remove them?
docs/ember/components.md
Outdated
| @@ -102,75 +127,6 @@ export default class ArgsDisplay extends Component { | |||
| </ul> | |||
| ``` | |||
|
|
|||
| ### Understanding `args` | |||
There was a problem hiding this comment.
Let's go ahead and leave this section (I'll do a follow-up on it to clean it up further). If you can just drop it back in, the rest of it looks like a great first iteration on this update, and will already be a big improvement for users. Thank you!
There was a problem hiding this comment.
I've added that section back in. I also started to update the section but got out of my depth pretty quickly so will leave the rest with you 🤣
There was a problem hiding this comment.
Haha that's exactly what I suggested this narrower path. Thank you!
docs/ember/components.md
Outdated
| @@ -133,44 +156,6 @@ let b = ["hello", "goodbye"]; // Array<string> | |||
|
|
|||
| In the case of the Component, we have the types the way we do so that you can’t accidentally define `args` as a string, or `undefined` , or whatever: it _has_ to be an object. Thus, `Component<Args extends {}>` . But we also want to make it so that you can just write `extends Component` , so that needs to have a default value. Thus, `Component<Args extends {} = {}>`. | |||
|
|
|||
| ### Giving `args` a type | |||
There was a problem hiding this comment.
Whoops, looks like this needs to be added back in as well!
docs/ember/components.md
Outdated
| Looking at that example above, Typescript knows what types `this.args` has, but how? In the `constructor` version, we explicitly _named_ the type of the `args` argument. Here, it seems to just work automatically. This works because the type definition for a Glimmer component looks roughly like this: | ||
|
|
||
| ```typescript | ||
| export default class Component<Args extends {} = {}> { | ||
| readonly args: Args; | ||
|
|
||
| constructor(owner: unknown, args: Args); | ||
| } | ||
| ``` | ||
| ```typescript | ||
| export default class Component { | ||
| args: Readonly<Args>; | ||
| constructor(owner: unknown, args: Args); | ||
| } | ||
| ``` | ||
|
|
||
| {% hint style="info" %} | ||
| Not sure what’s up with `<Args>` _at all_? We highly recommend the [TypeScript Deep Dive](https://basarat.gitbooks.io/typescript/) book’s [chapter on generics ](https://basarat.gitbooks.io/typescript/docs/types/generics.html) to be quite helpful in understanding this part. | ||
| Not sure what’s up with `<Args>` or `<S>` _at all_? We highly recommend the [TypeScript Deep Dive](https://basarat.gitbooks.io/typescript/) book’s [chapter on generics ](https://basarat.gitbooks.io/typescript/docs/types/generics.html) to be quite helpful in understanding this part. |
There was a problem hiding this comment.
Let's just leave these as they were as well, despite the slight incoherence. I will do a follow-up PR later today after merging yours which updates all of this language (and possibly does some restructuring as well).
There was a problem hiding this comment.
All done and both sections are back in their original state. Thanks!
This PR follows feedback from @chriskrycho on Ember Discuss, and aims to reflect the introduction of Component Signatures.
The changes focus on defining a Component Signature and the use of
Args. I've removed several sections that related toargsbeing an empty object. Now that defining a Signature is a prerequisite, they seemed less pertinent.After making these changes I spotted an existing PR that goes into detail about
ElementandBlock. I didn't attempt this because it felt like a duplication of the Glint docs. I'm happy for this PR to be sidelined if the existing PR ticks more boxes!