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

Change how IntrinsicElements type is defined #4915

Closed
1 task
jkjustjoshing opened this issue Sep 29, 2022 · 5 comments · Fixed by #5098
Closed
1 task

Change how IntrinsicElements type is defined #4915

jkjustjoshing opened this issue Sep 29, 2022 · 5 comments · Fixed by #5098
Labels
- P2: nice to have Not breaking anything but nice to have (priority) feat: typescript Related to TypeScript (scope)

Comments

@jkjustjoshing
Copy link
Contributor

jkjustjoshing commented Sep 29, 2022

What version of astro are you using?

1.2.5

Are you using an SSR adapter? If so, which one?

No

What package manager are you using?

npm

What operating system are you using?

Mac

Describe the Bug

I'm trying to implement an as prop to an Astro component. This prop should accept an HTML element tag name, and the component should render as that component. For example:

---
type Props = {
  as: HTMLTagString, // how can I type this correctly?
  id: string,
  'class': string
}
const Ele = Astro.props.as
---
<Ele id={Astro.props.id} class:list={[ Astro.props.class, 'red' ])>
  <slot />
</Ele>
<style>
  .red {
    color: red;
  }
</style>

To type the as prop, I'm trying to use keyof astroHTML.JSX.IntrinsicElements. Ideally, this would give me a union of all the possible HTML elements that Astro supports. However, the definition for IntrinsicElements is all HTML elements, but it also includes a catchall at the end

// Allow for arbitrary elements
[name: string]: { [name: string]: any };

This causes typeof astroHTML.JSX.IntrinsicElements to resolve to the type string, which isn't specific enough to allow the id and class props on <Ele>.

Suggestion

Create a DefinedIntrinsicElements interface that only includes defined elements, and rewrite IntrinsicElements to be

interface IntrinsicElements extends DefinedIntrinsicElements {
  // Allow for arbitrary elements
  [name: string]: { [name: string]: any };
}

This way, my component can define its props with

as: keyof astroHTML.JSX.DefinedIntrinsicElements

and the passed prop will correctly be restricted to known HTML elements.

Link to Minimal Reproducible Example

(I couldn't figure out how to get StackBlitz to show red squiggle for type errors, but I believe this is set up correctly)
https://stackblitz.com/edit/github-mwmoww-zt2tqt?file=astro.config.mjs,src%2Fpages%2Findex.astro&on=stackblitz

Participation

  • I am willing to submit a pull request for this issue.
@matthewp
Copy link
Contributor

I will defer to @Princesseuh who is our TS guru but what you are saying seems reasonable to me.

@matthewp matthewp added the - P2: nice to have Not breaking anything but nice to have (priority) label Sep 29, 2022
@natemoo-re
Copy link
Member

natemoo-re commented Sep 29, 2022

I totally agree with this! I wonder if we could even make the [name: string] definition more strict to only allow custom elements with a -? New HTML elements don't happen very often. 😅

Also notable: I have been pushing for having some built-in type utilities for patterns like this. In this case, my hope is that you can import an HTMLTag tag type rather than directly using the astroHTML.JSX global. Would also be great to have a built-in Polymorphic component type for the as pattern as well, but that is blocked by supporting Props + generics in the Language Server (currently in progress)

@Princesseuh
Copy link
Member

Princesseuh commented Sep 29, 2022

The suggestion makes total sense to me! This is a good workaround until we provide built-in type utilities like Nate suggest.

If you're willing to, feel free to open a PR with this change. Otherwise, just let me know and I'll tackle it ASAP!

Thank you for creating this issue!

@matthewp matthewp added the feat: typescript Related to TypeScript (scope) label Oct 7, 2022
@RodrigoTomeES
Copy link

Hi,

Is there any progress with this problem? It would be amazing be able to do this. Props should be the posible props of as, this is possible in React doing this: ButtonProps<T> & Omit<ComponentPropsWithRef<T>, keyof ButtonProps<T>>

export interface Props {
  as?: keyof astroHTML.JSX.DefinedIntrinsicElements;
  class?: string;
}

const { class: className = "", as: Element = "a", ...props } = Astro.props;

Thanks!

@fredericoo
Copy link

fredericoo commented May 31, 2023

Hi,

Is there any progress with this problem? It would be amazing be able to do this. Props should be the posible props of as, this is possible in React doing this: ButtonProps<T> & Omit<ComponentPropsWithRef<T>, keyof ButtonProps<T>>

export interface Props {
  as?: keyof astroHTML.JSX.DefinedIntrinsicElements;
  class?: string;
}

const { class: className = "", as: Element = "a", ...props } = Astro.props;

Thanks!

If you want to allow the as prop to dictate the component's props, you can do something like:

type Props<
  T extends keyof astroHTML.JSX.DefinedIntrinsicElements = keyof astroHTML.JSX.DefinedIntrinsicElements
> = {
  as?: T;
  otherProp: string;
} & Omit<astroHTML.JSX.DefinedIntrinsicElements[T], "slot">;

looks a bit dodgy, but any attempts to abstract it led to failure on my end. It works well with react because you can type the component itself, and not just props.

Astro 2.5 released some utilities for polymorphic components but they do not work for me, so I’m doing this instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: nice to have Not breaking anything but nice to have (priority) feat: typescript Related to TypeScript (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants