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

[Feature] Bundle use client with components #137

Open
colbyfayock opened this issue Feb 20, 2023 · 7 comments · May be fixed by #523
Open

[Feature] Bundle use client with components #137

colbyfayock opened this issue Feb 20, 2023 · 7 comments · May be fixed by #523
Assignees

Comments

@colbyfayock
Copy link
Collaborator

Feature Request

Is your feature request related to a problem? Please describe.

To fix an issue with use client not appropriately being added when used in Next.js 13 app directory, the directive was pulled out of the bundle instead recommending adding the use client directive to the file.

This isn't ideal, where the hope would be someone could use the components in this library interchangeably without having to worry about that additional distinction

This was a regression when moving to tsup, where however it's being bundled / compiled isn't supported or generally working.

Describe the solution you'd like

use client should be bundled with the components that require it, at a minimum CldImage as it uses useState and I believe it's additionally needed due to the loader prop which is a function.

@hades200082
Copy link
Contributor

Function props can't be passed into client components.

We might need to give this one more thought

@colbyfayock
Copy link
Collaborator Author

colbyfayock commented Feb 20, 2023

Maybe I'm missing something but that's how the Image component works for this type of thing, by passing a function to the loader prop

https://nextjs.org/docs/api-reference/next/image#loader

@hades200082
Copy link
Contributor

hades200082 commented Feb 20, 2023

Next/Image is a server component.

You can't pass a function from a server component to a client component because functions can't be serialised.

Ideally the CldImage component could be refactored to remove the need for useState allowing it to be server rendered, at which time this whole "use client" issue goes away anyway.

In addition, it would make the whole thing more performant because the image would be pre-rendered in the page on the server rather than having to be hydrated, evaluated and rendered at page-load time in the browser.

I might take a look at that at some point if you don't get to it first.

@hades200082
Copy link
Contributor

@colbyfayock just noting here also that React 18 "Shared Components", once supported by NextJS might be another way forwards.

@colbyfayock
Copy link
Collaborator Author

ah got it, thanks for the clarification

currently state is being used in order to force an update to the Next Image component, particularly in the event that the component receives a 423 error, which means the image is processing, that we poll for the results, and refresh the image once it's processed

its helpful for tools like background removal

@colbyfayock
Copy link
Collaborator Author

colbyfayock commented Jul 11, 2023

looks like one option here is multiple entry points: https://github.com/timolins/react-hot-toast/blob/main/tsup.config.ts

could potentially provide a default export of a server component with a separate entrypoint that wraps that component with the clientside helper if it would gracefully work that way

@colbyfayock
Copy link
Collaborator Author

clarification on this thread - Next IMage is a Client component as of July 17th: vercel/next.js#41924 (comment)

the multiple entry points work well, going to spin up a draft PR, though id like to try to solve a way for everything to still be importable from the top level module

@colbyfayock colbyfayock mentioned this issue Feb 26, 2024
4 tasks
@colbyfayock colbyfayock linked a pull request Sep 27, 2024 that will close this issue
@colbyfayock colbyfayock self-assigned this Sep 27, 2024
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 a pull request may close this issue.

2 participants