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

RFC: Component interface #300

Merged
merged 2 commits into from
Sep 29, 2022
Merged

RFC: Component interface #300

merged 2 commits into from
Sep 29, 2022

Conversation

Princesseuh
Copy link
Member

  • Start Date: 2022-09-12
  • Status: Draft

Summary

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

  • Ability to document components
  • (Future) Ability to type slots and possibly other things an Astro component could need in the future in one consolidated interface

Links

@Jutanium
Copy link

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
Copy link
Member Author

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
Copy link

+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
Copy link
Member Author

Princesseuh commented Sep 13, 2022

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

Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@Princesseuh
Copy link
Member Author

Princesseuh commented Sep 13, 2022

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
Copy link
Member

delucis commented Sep 13, 2022

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
Copy link
Member

okikio commented Sep 14, 2022

@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
Copy link
Member Author

@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
Copy link
Member

okikio commented Sep 14, 2022

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

@Princesseuh
Copy link
Member Author

@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
Copy link
Member

okikio commented Sep 14, 2022

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
Copy link
Member Author

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

@Princesseuh Princesseuh merged commit 84dc9db into main Sep 29, 2022
@Princesseuh Princesseuh deleted the rfc-component-interface branch September 29, 2022 13:31
@jasikpark
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants