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(compiler-dom): should bail hoisted on break content with innerHTM… #1230

Closed

Conversation

underfin
Copy link
Member

@underfin underfin commented May 25, 2020

…L (eg.tables related tags)

fix #vitejs/vite#226

Bug Reson

Look like it's caused by innerHtml error with table related tags.

Sloution DOMParser(Fail)

 const div = "<div><span class=\"foo\"></span><span class=\"foo\"></span><span class=\"foo\"></span><span class=\"foo\"></span><span class=\"foo\"></span></div>"
    const doc = new DOMParser().parseFromString(div, "text/html")
    console.log(doc.body.innerHTML)
    console.log(div)
    console.log(div.trim() === document.body.innerHTML.trim())
    console.log(div.length, document.body.innerHTML.length)
// run result
<div><span class="foo"></span><span class="foo"></span><span class="foo"></span><span class="foo"></span><span class="foo"></span></div>
<div><span class="foo"></span><span class="foo"></span><span class="foo"></span><span class="foo"></span><span class="foo"></span></div>
false
136 4023

I think can be parse by DOMParser, but it meet error for normal case.This is brower test result. And other run result in jest jsdom.

<div><span class="\&quot;foo\&quot;"></span><span class="\&quot;foo\&quot;"></span><span class="\&quot;foo\&quot;"></span><span class="\&quot;foo\&quot;"></span><span class="\&quot;foo\&quot;"></span></div>

@underfin
Copy link
Member Author

underfin commented Jun 3, 2020

#1286
Todo: This pr is used the value of context.parent is error.This above pr implement it correctly.

@yyx990803 yyx990803 closed this in a938b61 Jun 9, 2020
@yyx990803
Copy link
Member

  • context.parent is only usable during the normal transform phase. Static hoisting phase is a post-transform so context.parent is not accurate in this case.

  • No need to bail on p because anything can be placed inside p can be placed in div. Also colspan is not an element.

  • It should be enough to just check each root hoisted node for bail. The only case where it would produce unexpected result is when the user has authored invalid HTML in the first place. We can add a dev only check to ensure all stringified HTML structure is valid.

@underfin underfin deleted the fix-innerhtml-error-with-hoist branch June 10, 2020 01:21
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.

2 participants