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.

Sign up for GitHub

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

shadcn React 19 #1243

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
+219 −265
Open

shadcn React 19 #1243

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1a4fa4d
button
ahkhanjani Nov 8, 2024
084c61d
dropdown
ahkhanjani Nov 9, 2024
bc2c52c
form
ahkhanjani Nov 9, 2024
edbc673
input
ahkhanjani Nov 9, 2024
891e1d4
label
ahkhanjani Nov 9, 2024
7f616d4
toast
ahkhanjani Nov 9, 2024
bcd3213
make refs optional
ahkhanjani Nov 9, 2024
a8c8548
Merge branch 'main' into shadcn-react19
ahkhanjani Nov 9, 2024
1544145
clean up a type
ahkhanjani Nov 11, 2024
28ca02b
lots of cleaning up
ahkhanjani Nov 15, 2024
31f5fb4
cleanup
ahkhanjani Nov 15, 2024
21bc5bd
revert using radix's built in prop types
ahkhanjani Nov 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
button
@ahkhanjani
ahkhanjani committed Nov 8, 2024
commit 1a4fa4d433eaa20b9ed26d46bcf6ecd4f399eed2
31 changes: 18 additions & 13 deletions packages/ui/src/button.tsx
View file
Original file line number Diff line number Diff line change
@@ -38,21 +38,26 @@ const buttonVariants = cva(
interface ButtonProps
extends React.ButtonHTMLAttributes<HTMLButtonElement>,
VariantProps<typeof buttonVariants> {
ref: React.Ref<HTMLButtonElement>;
asChild?: boolean;
}

const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
({ className, variant, size, asChild = false, ...props }, ref) => {
const Comp = asChild ? Slot : "button";
return (
<Comp
className={cn(buttonVariants({ variant, size, className }))}
ref={ref}
{...props}
/>
);
},
);
Button.displayName = "Button";
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, I'm curious as to why we don't need displayName anymore. (tbh I don't know the purpose of it in the first place) do you have any resources to share about this?

Copy link
Member

Choose a reason for hiding this comment

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

From what I understand it's cause the component function inside the forwardRef previously was an anonymous function. you could have done forwardRef(function Button(...)) and have the displayName inferred before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Same goes for memo().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(tbh I don't know the purpose of it in the first place)

https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/display-name.md

DisplayName allows you to name your component. This name is used by React in debugging messages.

function Button({
ref,
className,
variant,
size,
asChild = false,
...props
}: ButtonProps) {
const Comp = asChild ? Slot : "button";
return (
<Comp
className={cn(buttonVariants({ variant, size, className }))}
ref={ref}
{...props}
/>
);
}

export { Button, buttonVariants };