withastro / roadmap

Ideas, suggestions, and formal RFC proposals for the Astro project.
292 stars 29 forks source link

RFC: Component interface #300

Closed Princesseuh closed 1 year ago

Princesseuh commented 2 years ago

Summary

Introduces a new interface called "Component" that is used to describe components. This unlocks the following benefits:

Links

Jutanium commented 2 years ago

Love this idea. The slots typing gets back to some awkwardness I've encountered: how do you accurately type slots and Elements in Astro?

Princesseuh commented 2 years ago

Love this idea. The slots typing gets back to some awkwardness I've encountered: how do you accurately type slots and Elements in Astro?

Admittedly, the slots typing part isn't really part of this RFC as much as this RFC unlocks at least room for a place to type slots 😛

Enteleform commented 2 years ago

+1, looks good!
 

(Future) Ability to type slots and possibly other things

I think due to the possibility for growth, the Component implementation might be preferable to the fragmented implementation in Alternatives.

For example, in a scenario where you're extending a common interface in multiple .astro files, you would only have to make updates to SomeCommonInterface.ts rather than going into each .astro file to check whether it has appropriate definitions for all of the specific fragments that now exist/don't-exist.

 

In the final implementation, would this be providing more thorough types for props?

e.g. in the case of Example:

function Button(props:{name:string}): any
Princesseuh commented 2 years ago

In the final implementation, would this be providing more thorough types for props?

e.g. in the case of Example:

function Button(props:{name:string}): any

Yes, just like the Props interface does at the moment. The result signature would be the following when the Component interface exists:

function Button(_props: Component.props): any
Princesseuh commented 2 years ago

Love how this is coming together! Would we also be able to ship strict typing for props if a user opts-into the new Component interface?

I am not 100% sure if I get what you mean. If you mean typing Astro.props strictly using Component.props, yes, that's possible, no issue there

If you mean making sure the shape of Component (or Component.props) is conform, I do not think so as it is still the definition of a new interface at the end of the day. This is also a strength in some way since it allows users to add new properties to their Component definition.

delucis commented 2 years ago

Love this @Princesseuh! Definitely has a nice migration story too: keep supporting Props and when a user is ready they move their Props into Component. Can’t wait to use this!

okikio commented 2 years ago

@Princesseuh Would it be possible to add export to the Component interface? It'd make it clear that the interface is meant for outside leaning tools.

e.g.

/**
 * @name Button
 * Description
 *
 * @slot default - Default slot
 * @slot button - Change the button
 * @slot ...
 */
export interface Component {
  // ...
}
Princesseuh commented 2 years ago

@Princesseuh Would it be possible to add export to the Component interface? It'd make it clear that the interface is meant for outside leaning tools.

e.g.

/**
 * @name Button
 * Description
 *
 * @slot default - Default slot
 * @slot button - Change the button
 * @slot ...
 */
export interface Component {
  // ...
}

It'd work just like the current Props interface so exporting it would be left to the user's choice (it works either way)

Personally, I think it's better to use normal rules for it and only export it when you actually need it in another file. But, I'm happy to support things either way!

okikio commented 2 years ago

@Princesseuh What do you think about using @slot to document slot?

Princesseuh commented 2 years ago

@Princesseuh What do you think about using @slot to document slot?

I'd rather not introduce additional rules to normal JSDoc comments to avoid incompatibility with external tools such as documentation generators

I think the format I'd recommend is a JSDoc comment on top of the actual definition of each slots - like you'd do for props right now.

(Should be noted that typing slots and the syntax for them isn't really in the scope of this RFC, it was just meant as an example of how this can be extended)

okikio commented 2 years ago

So, if I'm understanding you correctly, when a user has both Component, Props and Slots the language tool will prioritize Component, as users may want to define Props and/or Slots seperately/to add TSDoc comments to it, and then use it with Component

e.g.

/** 
 * ...
 */
interface Props {
  ...
}

/** 
 * ...
 */
interface Slots {
  ...
}

/**
 * @name Component
 * Description
 */
interface Components {
  slots: Slots,
  props: Props
}
Princesseuh commented 2 years ago

So, if I'm understanding you correctly, when a user has both Component, Props and Slots the language tool will prioritize Component, as users may want to define Props and/or Slots seperately/to add TSDoc comments to it, and then use it with Component

e.g.

/** 
 * ...
 */
interface Props {
  ...
}

/** 
 * ...
 */
interface Slots {
  ...
}

/**
 * @name Component
 * Description
 */
interface Components {
  slots: Slots,
  props: Props
}

That is right. For retrocompatibility sake, if the Component interface didn't exist in your example, the Props one would be used

jasikpark commented 1 year ago

Love how this is coming together! Would we also be able to ship strict typing for props if a user opts-into the new Component interface?

I am not 100% sure if I get what you mean. If you mean typing Astro.props strictly using Component.props, yes, that's possible, no issue there

If you mean making sure the shape of Component (or Component.props) is conform, I do not think so as it is still the definition of a new interface at the end of the day. This is also a strength in some way since it allows users to add new properties to their Component definition.

I'd assume this means transitioning to the default type of Astro.props being Component.props instead of Props & {}

i.e. not merging in the Record<string, any> type and requiring all props and slots to be explicit