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

Consolidate switch%platform, browser_only and platform #187

Open
jchavarri opened this issue Nov 28, 2024 · 5 comments
Open

Consolidate switch%platform, browser_only and platform #187

jchavarri opened this issue Nov 28, 2024 · 5 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@jchavarri
Copy link
Contributor

jchavarri commented Nov 28, 2024

There's now a bit confusing situation:

  • browser_only extension
  • switch%platform which include Server and Client branches
  • platform which includes js and native options
  • server-reason-react.browser_ppx uses -js to discard stuff for (modes melange)

We could use React namings and semantics to make things easier for folks to understand.

Maybe we could do something like this?

  • Remove platform as it's covering the same functionality as browser_only
  • Rename browser_only to just client
  • Add server to cover the platform native case
  • Change switch%platform to switch%environment

So to summarize, we'd have:

  • switch%environment with Server and Client branches
  • let%client
  • let%server

Wdyt?


Edited by @davesnx added server-reason-react.browser_ppx` example

@davesnx
Copy link
Member

davesnx commented Dec 3, 2024

let%client and let%server can potentially collide with RSC ppx design thought. Personally, the _only suffix makes the intention clearer, so would rather keep let%client_only and server_only

@davesnx davesnx added enhancement New feature or request help wanted Extra attention is needed labels Dec 3, 2024
@jchavarri
Copy link
Contributor Author

jchavarri commented Dec 4, 2024

let%client and let%server can potentially collide with RSC ppx design thought.

can you elaborate please? Edit: oh, i see, that we can use let%client as the OCaml version of use client and such. But it's not a conflict right? If you use use client then you only want to bundle that code only on the client which is what let%client would do.

Or in another way. Let's say we add let%client_only for this, and let%client for RSC. I guess people will ask "what's the difference between let%client_only and let%client and then we have to explain all implementation specific details, "this is Melange/OCaml boundary", "this is RSC server/client boundary" etc. I'd try to simplify as much as possible.

@davesnx
Copy link
Member

davesnx commented Dec 4, 2024

We totally agree, but we might not add let%client for RSC.

If we can align both, it would be massive, but I'm afraid that isn't the case. Current browser_only discards functions and use client just changes react components.

We could make use client do all the work of browser_only (inside the module), but we can't remove browser_only since functions that aren't react components will also need it. The other interesting bit is that client components need the server implementation as well. which makes use client a bit to do.

@davesnx
Copy link
Member

davesnx commented Dec 4, 2024

Worth sharing that there's a package called server-only that you import at the toplevel to enforce your bundler to not include those functions as server actions https://www.npmjs.com/package/server-only

And causes those kind of questions: https://www.reddit.com/r/nextjs/comments/18ih6nd/useserver_vs_serveronly/

@jchavarri
Copy link
Contributor Author

we can't remove browser_only since functions that aren't react components will also need it

to clarify, i don't suggest to remove it but rather rename it so there's no different "concepts" going around (e.g. "browser", "client").

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants