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

refactor(nuxt): don't wrap server placeholders/client fallbacks #21980

Merged
merged 8 commits into from Sep 13, 2023

Conversation

huang-julien
Copy link
Member

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Hi πŸ‘‹

This PR adds a small feature which i don't think will be used by many.

noClientOnlyTransform will prevent a client-side component to be wrapped within a createClientOnly.

As i'm currently working on a server components rework, this may be used (not a 100% sure) but it can be good to have this option as NuxtClientFallback will be using this instead of hard-coding the prevent behavior.
This can also be needed for some very advanced usages from modules author to avoid having some server code within client bundle due to possible side-effects (vue/server-renderer with NuxtClientFallback) by having a .server and .client component.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@stackblitz
Copy link

stackblitz bot commented Jul 6, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@@ -251,7 +251,8 @@ async function initNuxt (nuxt: Nuxt) {
name: 'NuxtClientFallback',
priority: 10, // built-in that we do not expect the user to override
filePath: resolve(nuxt.options.appDir, 'components/client-fallback.client'),
mode: 'client'
mode: 'client',
noClientOnlyTransform: true
Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a better naming for this ?

Copy link
Member

Choose a reason for hiding this comment

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

prewrapped? raw?

Copy link
Contributor

Choose a reason for hiding this comment

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

raw might be confusing since vue has markRaw which you might use on a component https://vuejs.org/api/reactivity-advanced.html#markraw

Copy link
Member Author

Choose a reason for hiding this comment

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

oh i'd prefer raw over prewrapped πŸ˜…

@huang-julien huang-julien marked this pull request as draft July 6, 2023 08:53
@huang-julien huang-julien marked this pull request as ready for review July 13, 2023 19:13
@danielroe danielroe added this to the 3.7 milestone Jul 14, 2023
Comment on lines 19 to 23
/**
* This prevent the component to be transformed with a `createClientOnly()` client-side if the mode is `client`
* An example is the `<NuxtClientFallback>` component which handles it's hydration client-side
*/
noClientOnlyTransform?: boolean
Copy link
Member

Choose a reason for hiding this comment

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

I would consider using this internally for NuxtClientFallback to test before making it a public property. Or do you think it's a common enough use-case that we need an API for it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure about that πŸ€” I guess we'd need to know if some users need it. It's mainly a way to keep side-effects modules only on 1 side (server or client).

Copy link
Member

Choose a reason for hiding this comment

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

I'd be happy to merge an fix needing this with an internal property that we set and consume for <NuxtClientFallback>, rather than exposing this as a public property for components which can be enabled by users ...

Copy link
Member Author

Choose a reason for hiding this comment

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

let's mark it private for now πŸ‘
i think @internal should be enough to hide it ? πŸ€”

@danielroe danielroe removed this from the 3.7 milestone Aug 25, 2023
@danielroe danielroe changed the title feat(nuxt): add noClientOnlyTransform to Component refactor(nuxt): mark server placeholders + NuxtClientFallback as raw Sep 13, 2023
@danielroe danielroe changed the title refactor(nuxt): mark server placeholders + NuxtClientFallback as raw refactor(nuxt): don't wrap server placeholders/client fallbacks Sep 13, 2023
@danielroe danielroe merged commit 95d1f99 into nuxt:main Sep 13, 2023
26 checks passed
@github-actions github-actions bot mentioned this pull request Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants