shadcn-ui / ui

Beautifully designed components that you can copy and paste into your apps. Accessible. Customizable. Open Source.
https://ui.shadcn.com
MIT License
75.9k stars 4.77k forks source link

'className' does not exist on type 'DialogPortalProps' #1595

Closed basilaslam closed 1 year ago

basilaslam commented 1 year ago

typescript throwing "Property 'className' does not exist on type 'DialogPortalProps'" when using shadcn SheetPortal component image

rafaelsilva81 commented 1 year ago

Same here, looks like a problem on the typings of the DialogPortal

DialogPortalProps should extend PortalProps here: image

Or it should be like this: image

basilaslam commented 1 year ago

Yes but i think shadcn is using the types from redix-ui

rafaelsilva81 commented 1 year ago

Yes, the problem is in radix-ui, more specifically @radix-ui/react-dialog on version v1.0.5

If you remove the ^ from your package.json, delete your lock file, run install again and stick with v1.0.4 for now the problem is solved. They were probably not expecting anyone putting classes into the portal, and then pushed this small update, but its used here.

basilaslam commented 1 year ago

I temporarily resolved the issue by removing the className from both the props and the sheetPrimitive.portal component. image Although this solution has worked for now, I am uncertain if it might cause problems in the future. However, as of now, my project is functioning correctly.

rafaelsilva81 commented 1 year ago

Also works, if you're not using any classnames on your portals anywhere.

I'm a little bit confused if this is something that should be fixed on Radix's end or here. On one side, I don't think Portals should have any classnames, but this is probably going to break some peoples projects

basilaslam commented 1 year ago

Thanks for your assistance, @rafaelsilva81. I also don't think Portals should have any classnames.

Now, I'm wondering whether I should go ahead and close this issue, or if there's any reason to keep it open. What would you recommend?

rafaelsilva81 commented 1 year ago

Glad to help. I'd say leave it open for now, maybe it is a change we need to make here. But I've opened a similar issue on the radix-ui repo just in case

https://github.com/radix-ui/primitives/issues/2410

tecoad commented 1 year ago

I confirm this doesnt ot only happen on @radix-ui/react-dialog, but also on @radix-ui/react-alert-dialog. Obviously its radix mess, since removing a prop could be a breaking change, shouldn't be a minor change in my opinion.

rafaelsilva81 commented 1 year ago

I confirm this doesnt ot only happen on @radix-ui/react-dialog, but also on @radix-ui/react-alert-dialog. Obviously its radix mess, since removing a prop could be a breaking change, shouldn't be a minor change in my opinion.

Agree, but in case its something the radix team doesn't want to fix, we can change it here either by removing the className property or adding it manually. I can see why they wouldn't want classNames on this component, but it really shouldn't be something pushed in a minor update.

chof64 commented 1 year ago

I'm getting this error on the Alert Dialog and Sheet components on my project.

As mentioned above, I've used the exact versions, @radix-ui/react-dialog@1.0.4 and @radix-ui/react-alert-dialog@1.0.4 as a workaround, by running yarn add --exact @radix-ui/react-dialog@1.0.4 @radix-ui/react-alert-dialog@1.0.4

Lenghak commented 1 year ago

@chof64 Thanks to you, this solution works. But instead of yarn, I use bun as my package manager. I was wondering why the param --exact work. I mean what does it do and why normal installation does not work?

rafaelsilva81 commented 1 year ago

@Lenghak Normal installation will try to install the latest version, which is bugged. exact guarantees the version which is not bugged

Lenghak commented 1 year ago

I see. Then maybe I have done something wrong with my pacakge.json when I tried to manually edit the version and reinstall it. Anyway the problem is fixed, and really appreciate your help.

rafaelsilva81 commented 1 year ago

Editing the package.json should work, maybe you forgot to remove the ^ to guarantee the exact version. Or maybe you didn't delete you node_modules and your lock file (in this case bun.lock I think) to ensure the desired version was installed

Lenghak commented 1 year ago

Ahh. Thank you for the tips, though. I will try that next time.

damiensedgwick commented 1 year ago

Also getting this error when upgrading the following:

@radix-ui/react-dialog 1.0.4 ❯ 1.0.5

Patch changes should not be API breaking changes! This is irrelevant of whether a portal should have classNames or not in my opinion.

lachieh commented 1 year ago

Agreed that the change in https://github.com/radix-ui/primitives/pull/2178 should not have been a patch.

There are two temporary fixes:

  1. Install the v1.0.4 version of radix with this command: npm i @radix-ui/react-dialog@1.0.4 --exact
  2. Remove the className property from the components

The more permanent solution is going to have to be a code change in this library, as it's unlikely that radix will be reverting the change.

joaocottabadaro commented 1 year ago

Same error here. Would the solution for radix devs be to do this? screenshotRadixComponent

nodegin commented 1 year ago

What is the best practice of syncing shadcn-ui with radix-ui? For upgrading shadcn-ui components we use diff command, but I wonder when it comes to a case like this, how can we upgrade radix-ui without breaking the component or know if shadcn-ui updated?

lachieh commented 1 year ago

In this instance, the Radix maintainers have acknowledged that the change to remove/correct the types for DialogProps should not have been a patch so this is an exceptional case.

Normally, the diff command is fine or simply installing the same component again with add should do the trick. The PR I opened (#1606) will resolve this, and then you'll be able to do either of these options though the temporary fix above should work

avi-l commented 1 year ago

Thanks!

DavidJMS commented 1 year ago

En el package.json hay que cambiar esto "@radix-ui/react-dialog": "^1.0.4", a esto otro "@radix-ui/react-dialog": "1.0.4", o la versión que estaban usando, esto para que no se actualice a la ultima version la cual podría tener modificaciones de incompatibilidad con versiones anteriores.

marinhomich commented 1 year ago

When changing package.json to "@radix-ui/react-dialog": "1.0.4". I can't click anywhere after using the AlertDialogCancel. Same here #468

weiwei2694 commented 1 year ago

I'm getting this error on the Alert Dialog and Sheet components on my project.

As mentioned above, I've used the exact versions, @radix-ui/react-dialog@1.0.4 and @radix-ui/react-alert-dialog@1.0.4 as a workaround, by running yarn add --exact @radix-ui/react-dialog@1.0.4 @radix-ui/react-alert-dialog@1.0.4

Thankyou bro

mastoj commented 1 year ago

Very strange, but I have the same issue even though I've pinned radix-dialog to 1.0.4... The build complains about the className, but when digging through code it should be there. So I'm not sure what is going on here to be honest.

avi-l commented 1 year ago

The downgrade to 1.0.4 worked fine for me.

On Thu, Sep 28, 2023, 14:07 Tomas Jansson @.***> wrote:

Very strange, but I have the same issue even though I've pinned radix-dialog to 1.0.4... The build complains about the className, but when digging through code it should be there. So I'm not sure what is going on here to be honest.

— Reply to this email directly, view it on GitHub https://github.com/shadcn-ui/ui/issues/1595#issuecomment-1738940296, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQHF5MPHETJY743QGWO4PTLX4VLA3ANCNFSM6AAAAAA5GM4UNE . You are receiving this because you commented.Message ID: @.***>

lachieh commented 1 year ago

Very strange, but I have the same issue even though I've pinned radix-dialog to 1.0.4... The build complains about the className, but when digging through code it should be there. So I'm not sure what is going on here to be honest.

@mastoj take a look at the PR #1606 and make the same code changes to whatever components you are using. The types were corrected to remove className because you can't apply a className to something that doesn't have a DOM element.

devlulcas commented 1 year ago

I was having the same problema with the components based on the dialog component

Errors  Files
     2  src/shared/components/ui/dialog.tsx:13
     2  src/shared/components/ui/sheet.tsx:16

Downgrading @radix-ui/react-dialog to 1.0.4 worked for me aswell. Thank you all

nazimboudeffa commented 1 year ago

I have the same problem but not in dev environement only on Vercel It worked by doing npm remove @radix-ui/react-dialog then npm install @radix-ui/react-dialog@1.0.4 --exact Maybe also @radix-ui/react-alert-dialog v1.0.4

chk7964 commented 1 year ago

my problem fix by adding this package "@radix-ui/react-dialog": "1.0.4"

tausif-fardin commented 1 year ago

The downgrade to 1.0.4 worked fine for me.

The first one worked for me. Thanks

dev-aligh commented 1 year ago

Yes, the problem is in radix-ui, more specifically @radix-ui/react-dialog on version v1.0.5

If you remove the ^ from your package.json, delete your lock file, run install again and stick with v1.0.4 for now the problem is solved. They were probably not expecting anyone putting classes into the portal, and then pushed this small update, but its used here.

thanks it's worked ❤️

rtorcato commented 1 year ago

temp fix with ts-ignore

// @ts-ignore
const SheetPortal = ({ className, ...props }: SheetPrimitive.DialogPortalProps & { className?: string }) => <SheetPrimitive.Portal className={cn(className)} {...props} />
SheetPortal.displayName = SheetPrimitive.Portal.displayName

const AlertDialogPortal = ({ className, children, ...props }: AlertDialogPrimitive.AlertDialogPortalProps & { className?: string }) => (
  // @ts-ignore
  <AlertDialogPrimitive.Portal className={cn(className)} {...props}>
    <div className="fixed inset-0 z-50 flex items-end justify-center sm:items-center">{children}</div>
  </AlertDialogPrimitive.Portal>
)
luanrem commented 1 year ago

I'm having the same problem. The build failed.
Even removing the ^ from the package.json didn't work, the build still failing.

rtorcato commented 1 year ago

I'm having the same problem. The build failed. Even removing the ^ from the package.json didn't work, the build still failing.

removing the ^ won't update your package in node_modules. You can remove node_modules folder and delete pnpm.lock or npm.lock file and install again after removing ^

lachieh commented 1 year ago

This problem is very easily solvable by editing the components to remove the use of className. Take a look at the code in the PR (#1606) for reference.

Downgrading to a fixed package version will mean you won't get newer patch updates, and using @ts-ignore is an anti-pattern.

AbhayaShankar commented 1 year ago

Yes removing className from DialogPortal works. Its not showing any errors now. This should be updated in the newer versions of shadcn/ui library.

Same issue is with SheetPortal as well. there as well I got same error, removing className fixes the error

raphtlw commented 1 year ago

quickfix: either re-download the dialog file from shadcn-ui@latest add dialog or re-write the DialogPortal as such:

const DialogPortal = DialogPrimitive.Portal
lachieh commented 1 year ago

This is resolved by #1603 which was released in 0.4.1. To resolve, run:

npx shadcn-ui@latest add dialog
GiantappMan commented 1 year ago

The dialog is fixed, but the sheet still has the same issue.

./src/components/ui/sheet.tsx:17:3

Type error: Property 'className' does not exist on type 'DialogPortalProps'.

  15 | 
  16 | const SheetPortal = ({
> 17 |   className,
     |   ^
  18 |   ...props
  19 | }: SheetPrimitive.DialogPortalProps) => (
  20 |   <SheetPrimitive.Portal className={cn(className)} {...props} />
 ELIFECYCLE  Command failed with exit code 1.
lachieh commented 1 year ago

Ok, thanks for the heads up. #1606 should resolve this then.

HoshangDEV commented 1 year ago

I still have this issue while delpoying the project in Vercel, first i had the problem in my code and also in Vercel, but after downgrading to "@radix-ui/react-dialog": "1.0.4" the error fixed in my VS Code, but still getting it in Vercel while deploying the project:

./components/ui/sheet.tsx:17:3
--
19:29:49.250 | Type error: Property 'className' does not exist on type 'DialogPortalProps'.
19:29:49.250 |  
19:29:49.250 | 15 \|
19:29:49.250 | 16 \| const SheetPortal = ({
19:29:49.250 | > 17 \|   className,
19:29:49.250 | \|   ^
19:29:49.250 | 18 \|   ...props
19:29:49.250 | 19 \| }: SheetPrimitive.DialogPortalProps) => (
19:29:49.251 | 20 \|   <SheetPrimitive.Portal className={cn(className)} {...props} />
19:29:49.332 | Error: Command "npm run build" exited with 1
meoawww commented 1 year ago

changing the @radix-ui/react-dialog:"^1.0.5" to @radix-ui/react-dialog: "1.0.4" in package.json and npm install in terminal will solve this for now.

martmalo commented 1 year ago

changing the @radix-ui/react-dialog:"^1.0.5" to @radix-ui/react-dialog: "1.0.4" in package.json and npm install in terminal will solve this for now.

Thank you

mertafor commented 1 year ago

Almost all radix libraries that I installed have the same problem. Switch, checkbox, dialog, alert etc. has this classname doesn't exist issue but also standard props like name, id etc. are missing as well. I've had to downgrade a minor version all libraries to fix it.

According to the replies on Radix's github issue (https://github.com/radix-ui/primitives/issues/2410), this is intended behavior. Is it planned to release a fix or are we going to stuck in previous releases forever?

lachieh commented 1 year ago

Is it planned to release a fix or are we going to stuck in previous releases forever?

@mertafor the hyperbole in this comment could be seen as a little aggressive given that there was a new release within the last month and there is already quite a number of solutions listed in this thread. I suggest you think about how your comment could come across given that this is a free, open source project and all of the contributors are volunteers.

mertafor commented 1 year ago

Is it planned to release a fix or are we going to stuck in previous releases forever?

@mertafor the hyperbole in this comment could be seen as a little aggressive given that there was a new release within the last month and there is already quite a number of solutions listed in this thread. I suggest you think about how your comment could come across given that this is a free, open source project and all of the contributors are volunteers.

I'm sorry if I offended anyone but I believe it's a valid question since I didn't see any viable solutions except downgrading or removing classname altogether which doesn't work in case of forwardref (maybe unnecessarily wrapping the component would help). I appreciate any open source work and any contributor and sorry for poor choice of words. On the other hand I might be little upset because it's really difficult to find something does the job properly and all of sudden an unexpected problem ruins your day (I've spent some time to fix this problem)

However, the truth is if current behavior is intended, I don't see any other solutions than being stuck on an older release as other people did here, which is the harsh truth.

lachieh commented 1 year ago

Here is a recap for everyone since the thread is quite long.

@mertafor if you're having issues with components other than these three, I suggest you log another issue with a reduced test case to isolate the problem.

mertafor commented 1 year ago

@lachieh I appreciate taking your time and providing a recap. Updating current dialog and alert files according to npx generated new ones fixed problem for me.

manopriyan6 commented 1 year ago

Yes, the problem is in radix-ui, more specifically @radix-ui/react-dialog on version v1.0.5

If you remove the ^ from your package.json, delete your lock file, run install again and stick with v1.0.4 for now the problem is solved. They were probably not expecting anyone putting classes into the portal, and then pushed this small update, but its used here.

its Working Thank You For Your Kingd Information