Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

fix(nuxt): remove fragment from createClientOnly #7774

Merged
merged 11 commits into from
Oct 3, 2022

Conversation

huang-julien
Copy link
Member

@huang-julien huang-julien commented Sep 22, 2022

πŸ”— Linked issue

linked to #6165 (comment)
resolves nuxt/nuxt#15014, resolves nuxt/nuxt#15039

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 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)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Hi πŸ‘‹ πŸ‘‹ This PR remove the Fragment workaround which prevent using v-show on .client components even if it has a single root node.

There is additionnal changes in the setup part where we return the render function.

Previously in the linked PR, there's was a Oldchildren is null issue if we returned directly the setup result as a render function.
Using the h() function to return the VNode was working only for some components (based on @danielroe 's #6165 (comment)).

However the component fails to render or update its render silently if it only has a root tag with a string and a variable such as :

<template>
  <div>
      I'm a string children {{ count }}. I will be mounted but will still render a simple div
  </div>
</template>

The workaround was to use a Fragment but it leads to nuxt/nuxt#15014 issue .

This is fixed by verifying the result of the render function children which can be string|array|null|object and rendering the component using createElementVNode when res.children is null or a string

πŸ“ Checklist

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

@codesandbox
Copy link

codesandbox bot commented Sep 22, 2022

CodeSandbox logoCodeSandbox logoΒ  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

@netlify
Copy link

netlify bot commented Sep 22, 2022

βœ… Deploy Preview for nuxt3-docs ready!

Name Link
πŸ”¨ Latest commit 0e9ccd3
πŸ” Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/6331877876654c0008d564fd
😎 Deploy Preview https://deploy-preview-7774--nuxt3-docs.netlify.app
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@danielroe danielroe added bug Something isn't working πŸ”¨ p3-minor-bug Priority 3: a bug in an edge case that only affects very specific usage labels Sep 23, 2022 — with Volta.net
Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

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

Thank you! ❀️

This seems like a good fix to me. Is there a way we can add a test for the different kinds of client-only components? Maybe a page in the fixture with a variety of types and we can use createPage to assert that all content is correctly displayed?

@huang-julien huang-julien marked this pull request as draft September 23, 2022 13:58
@huang-julien
Copy link
Member Author

huang-julien commented Sep 23, 2022

I think this should now cover most of the issues we can run into.

let me know if i missed something :)

@huang-julien huang-julien marked this pull request as ready for review September 23, 2022 15:07
Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

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

This looks great, and I'm really happy with the test suite - thank you 😊

test/basic.test.ts Outdated Show resolved Hide resolved
@huang-julien huang-julien marked this pull request as draft September 26, 2022 09:38
@huang-julien
Copy link
Member Author

i forgot to set .client on some test components

@pi0 pi0 changed the title fix(nuxt): remove Fragment from createClientOnly fix(nuxt): remove fragment from createClientOnly Sep 26, 2022
@huang-julien
Copy link
Member Author

huang-julien commented Sep 26, 2022

updated #7774 (comment)

  • fixed missing .client in test components name.
  • Use createElementVNode instead of createElementBlock. Components without any state or script are not rendered using createElementBlock.
  • test V show directive reactivity using a ref

@huang-julien huang-julien requested review from danielroe and antfu and removed request for danielroe and antfu September 26, 2022 10:22
@huang-julien huang-julien marked this pull request as ready for review September 26, 2022 10:23
@huang-julien huang-julien requested review from antfu and danielroe and removed request for danielroe and antfu September 26, 2022 10:23
@atinux atinux merged commit e6ca07b into nuxt:main Oct 3, 2022
@danielroe danielroe mentioned this pull request Oct 9, 2022
@danielroe danielroe added the 3.x label Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3.x bug Something isn't working πŸ”¨ p3-minor-bug Priority 3: a bug in an edge case that only affects very specific usage
Projects
None yet
4 participants