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

Jump to bottom

[feat]: remove 'use client' from navigation-menu.tsx #6888

Closed
2 tasks done
benwaffle opened this issue Mar 7, 2025 · 8 comments · Fixed by #6913 · May be fixed by #6932
Closed
2 tasks done

[feat]: remove 'use client' from navigation-menu.tsx #6888

benwaffle opened this issue Mar 7, 2025 · 8 comments · Fixed by #6913 · May be fixed by #6932
Labels
area: request

Comments

@benwaffle
Copy link

Feature description

As far as I can tell, "use client" isn't necessary here, right? It seems to work fine.

Radix says all components are SSR-able.

Affected component/components

NavigationMenu

Additional Context

No response

Before submitting

  • I've made research efforts and searched the documentation
  • I've searched for existing issues and PRs
@Jacksonmills
Copy link
Contributor

looks like it's been removed, maybe you are still using an older version?

@benwaffle
Copy link
Author

I just ran pnpm dlx shadcn@latest add navigation-menu in a brand new app and it's still there.

@Jacksonmills
Copy link
Contributor

I'll test this today

@Jacksonmills
Copy link
Contributor

noticed it seems the v4 navigation menu still has 'use client' I can open a PR to remove

@Jacksonmills Jacksonmills mentioned this issue Mar 12, 2025
Merged
@benwaffle
Copy link
Author

Can we do the same for the following? From a simple grep, these components don't use any hooks but have "use client";.

accordion
alert-dialog
aspect-ratio
avatar
calendar
checkbox
collapsible
command
context-menu
dialog
drawer
dropdown-menu
hover-card
label
menubar
popover
progress
radio-group
resizable
scroll-area
select
separator
sheet
switch
table
tabs
toggle
tooltip

@Jacksonmills Jacksonmills mentioned this issue Mar 14, 2025
Open
@Jacksonmills
Copy link
Contributor

@benwaffle check out the PR, I found in testing each in a server only demo there were a couple that still require the 'use client' directive like Calendar

@benwaffle
Copy link
Author

nice!

Jacksonmills added a commit to Jacksonmills/ui that referenced this issue Mar 14, 2025
@Jacksonmills
fix(v3 ui): 6888 - Remove unnecessary 'use client' directives from UI…
5aace50 
… components

Removes redundant 'use client' directives from various UI component files to optimize client-side performance.

Fixes shadcn-ui#6888
@Jacksonmills Jacksonmills mentioned this issue Mar 14, 2025
Open
@Jacksonmills
Copy link
Contributor

Added v3 changes too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: request
Projects
None yet
Milestone
No milestone
2 participants
@benwaffle @Jacksonmills