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

fix: export ClassList for easier typing of components with a class prop #4114

Merged
merged 4 commits into from
May 8, 2023

Conversation

steve8708
Copy link
Contributor

@steve8708 steve8708 commented May 8, 2023

a common pattern in React is to take className as a prop, like

function MyComponent(props: { className?: string }) {
  return <div className={`${props.className || ''} my-class`}>...</div>
}

<MyComponent className="some-class" />

But Qwik supports a more elegant API for classes, but it is a bit awkward to work with the types for as the actual type is not exported.

So today I do this:

import type  { HTMLAttributes } from '@builder.io/qwik'

function MyComponent(props: { class?: HTMLAttributes<HTMLElement>["class"] }) {
  return <div class={[props.class, 'my-class']}>...</div>
}

but it would be much nicer to just do

import type  { ClassList } from '@builder.io/qwik'

function MyComponent(props: { class?: ClassList }) {
  return <div class={[props.class, 'my-class']}>...</div>
}

This PR exports ClassList so we can do this

What is it?

  • Feature / enhancement
  • Bug
  • Docs / tests / types / typos

Checklist:

  • My code follows the developer guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • Added new tests to cover the fix / functionality

a common pattern in React is to take className as a prop, like

```tsx
function MyComponent(props: { className?: string }) {
  return <div className={`${props.className || ''} my-class`}>...</div>
}
```

But Qwik supports a more elevant API for classes, but it is a bit awkward to work with the types for as the actual type is not exported

So today I do this:

```tsx
function MyComponent(props: { class?: HTMLAttributes<HTMLElement>["class"] }) {
  return <div class={[props.class, 'my-class']}>...</div>
}
```

but it would be much nicer to just do

```tsx
function MyComponent(props: { class?: ClassList }) {
  return <div class={[props.class, 'my-class']}>...</div>
}
```

This PR exports ClassList so we can do this
@stackblitz
Copy link

stackblitz bot commented May 8, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@manucorporat manucorporat enabled auto-merge (squash) May 8, 2023 17:50
Copy link
Contributor

@manucorporat manucorporat left a comment

Choose a reason for hiding this comment

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

love it!

@shairez
Copy link
Collaborator

shairez commented May 8, 2023

great PR @steve8708 ! this will be useful in Qwik UI as well

BTW, you can run pnpm fmt.staged and it'll fix the prettier error just for the staged changes (more convenient than running on everything)

@manucorporat manucorporat merged commit d77fa17 into QwikDev:main May 8, 2023
19 checks passed
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.

None yet

3 participants