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

fix(nuxt): exclude data-v attrs from server component props #23095

Merged
merged 8 commits into from Sep 10, 2023

Conversation

huang-julien
Copy link
Member

πŸ”— Linked issue

fix #23051

❓ 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 resolves #23051 .

I'm not really a fan of having to make a computed + a reduce, but i didn't find another solution atm. Let's admit that server components are not going to be updated every milliseconds πŸ˜… .... If there's a better solution (like a config from vue), let me know.

This only affect .server components since NuxtIsland directly use props props attrbute and ignore any data-v attribute

πŸ“ Checklist

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

@stackblitz
Copy link

stackblitz bot commented Sep 8, 2023

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

@huang-julien huang-julien changed the title fix(nuxt): avoid sending down data-v attributes of cssLoader to NuxtIsland fix(nuxt): avoid sending down data-v attributes to NuxtIsland Sep 8, 2023
@huang-julien huang-julien changed the title fix(nuxt): avoid sending down data-v attributes to NuxtIsland fix(nuxt): avoid sending data-v attributes to NuxtIsland Sep 8, 2023
@@ -7,10 +7,12 @@ export const createServerComponent = (name: string) => {
inheritAttrs: false,
props: { lazy: Boolean },
setup (props, { attrs, slots }) {
// #23051 - remove data-v attributes
const attrsWithoutVueDataAttr = computed(() => Object.entries(attrs).reduce<Record<string, string>>((acc, [key, value]) => key.startsWith('data-v-') ? acc : Object.assign(acc, { [key]: value }), {}))
Copy link
Member

Choose a reason for hiding this comment

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

I think we should do this within the render function, and I think it would likely be simpler to do Object.fromEntries + Object.entries().filter.

@huang-julien huang-julien marked this pull request as draft September 9, 2023 18:27
@huang-julien huang-julien marked this pull request as ready for review September 9, 2023 18:40
@arsengoian
Copy link
Contributor

Apologies, but if it's fixed by not passing the data-v attribute, won't this break styles during SSG? I think that might be a bigger problem actually, since that means that scoped styles won't apply during SSG and if you want to style server components used inside your components with scoped styles you'll have to use :deep()

I mean, this can break styles in already existing applications, no?

@huang-julien huang-julien changed the title fix(nuxt): avoid sending data-v attributes to NuxtIsland fix(nuxt): avoid sending data-v attributes to NuxtIsland props Sep 9, 2023
@huang-julien
Copy link
Member Author

Oh the title of the PR may not be explicit enough.
The issue is that we sent some unwanted attribute as props to NuxtIsland, thus causing the hash to change with SSG.

@arsengoian
Copy link
Contributor

Thanks for the clarification!

@danielroe danielroe changed the title fix(nuxt): avoid sending data-v attributes to NuxtIsland props fix(nuxt): exclude data-v attrs from server component props Sep 10, 2023
@danielroe danielroe merged commit 3f9fa00 into nuxt:main Sep 10, 2023
26 checks passed
@github-actions github-actions bot mentioned this pull request Sep 10, 2023
arsengoian added a commit to arsengoian/nuxt that referenced this pull request Sep 24, 2023
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 this pull request may close these issues.

Nuxt3 .server (island) components fail to render on soft navigation if they have different props on pages
3 participants