Skip to content
This repository was archived by the owner on Apr 6, 2023. It is now read-only.
/frameworkPublic archive

feat(nuxt): parse html to treeshake client-only components - #7527

Merged
danielroe merged 10 commits into
mainfrom
fix/parse-client-html
Oct 10, 2022
Merged

feat(nuxt): parse html to treeshake client-only components#7527
danielroe merged 10 commits into
mainfrom
fix/parse-client-html

Conversation

@danielroe

Copy link
Copy Markdown
Member

🔗 Linked issue

resolvesnuxt/nuxt#14896

❓ 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

This switches from regexp -> parse5 (fast HTML parsing) to remove client only component nodes. It's more stable and also allows us to preserve fallback slots, for example.

The blocker for this PR is currently unjs/jiti#84; jiti is having trouble transforming parse5, blocking testing.

📝 Checklist

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

@danielroedanielroe added bug Something isn't working enhancement New feature or request 🔨 p3-minor-bug Priority 3: a bug in an edge case that only affects very specific usage labels Sep 14, 2022
@danielroedanielroe self-assigned this Sep 14, 2022
@netlify

netlifyBot commented Sep 14, 2022

Copy link
Copy Markdown

Deploy Preview for nuxt3-docs canceled.

NameLink
🔨 Latest commit83bf655
🔍 Latest deploy loghttps://app.netlify.com/sites/nuxt3-docs/deploys/633051aa1e602c0009eca831

@pi0

pi0 commented Sep 15, 2022

Copy link
Copy Markdown
Member

I will look into jiti issue. Isn't there an easy way to fix the regex issue? Parse5, even if super fast is stil parsing the strings. We might use a hybrid method (like mlly) to improve precision.

@danielroe

Copy link
Copy Markdown
MemberAuthor

Indeed I could fix that particular issue with regexp but I feel parsing the HTML is more reliable. For example, there was always the caveat of nested client-only components.

At some point you reinvent the parser. And I believe parse5 will soon be exposed to us by vite, so we can even benefit from vite caching ast (iirc): vitejs/vite#9678

@pi0

pi0 commented Sep 15, 2022

Copy link
Copy Markdown
Member

Yes, I'm aware that vite itself is using parse5 and it is a really good decision.

But here we are (re) parsing the same template code into AST in a plugin for a particular optimization that benefits only some components. Full parsing makes the built-time overall slower.

If there are finally more edge cases that we cannot handle with regex only, same as mlly we can use a two phase detection with quick regex matcher and precision AST (or Tokenized) verifier when we really are going to make a change to the code.

It would be nice if first could resolve the linked issue via regex and keep working on this PR as a general enhancenment.

@pi0

pi0 commented Sep 19, 2022

Copy link
Copy Markdown
Member

@danielroe Anyway we can workaround nuxt/nuxt#14896 regex issue?

@danielroe

Copy link
Copy Markdown
MemberAuthor

~> #7659

@danielroe

Copy link
Copy Markdown
MemberAuthor

Updating this PR just to keep it in sync. You were right to be careful; parse5 doesn't handle arbitrary self-closing elements. I suppose we could use an xml parser, but a big appeal of it for me was aligning with vite.

@danielroe

Copy link
Copy Markdown
MemberAuthor

🚀 Switched to ultrahtml, a 1.75kB library with zero dependencies and deliberate support for parsing Vue.

@danielroe
danielroe marked this pull request as ready for review September 25, 2022 12:59
@danielroe
danielroe requested a review from pi0September 25, 2022 12:59
@danielroedanielroe mentioned this pull request Oct 9, 2022
@danielroe
danielroe requested review from antfu and atinuxOctober 10, 2022 10:14
@danielroe
danielroe merged commit 26b1c9c into mainOct 10, 2022
@danielroe
danielroe deleted the fix/parse-client-html branch October 10, 2022 15:48
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

3.xbugSomething isn't workingenhancementNew feature or request🔨 p3-minor-bugPriority 3: a bug in an edge case that only affects very specific usage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

<ClientOnly> with <template #fallback> always results in hydration error in RC10

3 participants

@danielroe@pi0@atinux