Skip to content

remove comment dom node #1342

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

Closed
tangjinzhou opened this issue Jun 11, 2020 · 11 comments
Closed

remove comment dom node #1342

tangjinzhou opened this issue Jun 11, 2020 · 11 comments

Comments

@tangjinzhou
Copy link
Contributor

Version

3.0.0-beta.14

Reproduction link

https://jsbin.com/vucokaroxo/1/edit?html,output

Steps to reproduce

null and undefined will generate comment dom.

image

What is expected?

remove it

What is actually happening?

have comment dom

@lawvs
Copy link
Contributor

lawvs commented Jun 11, 2020

It might be expected

vuejs/vue#5117 (comment)

v-if is usually used for elements in a relatively stable node structure, rendering it to empty comment tags makes vnode lists diffing more efficient as the lists are more "stable", and it avoids some edge cases when elements are not keyed. In your case it doesn't seem to cause any real issue other than small inspection annoyance, so unfortunately I don't think we would change this behavior.

// https://github.com/vuejs/vue-next/blob/master/packages/runtime-core/src/vnode.ts#L491-L493
if (child == null || typeof child === 'boolean') {
  // empty placeholder
  return createVNode(Comment)
} else if (isArray(child)) {

@underfin
Copy link
Member

underfin commented Jun 11, 2020

@lawvs .That scene is underv-if create comment.This issues really is inconsistent with 2.x.

@yyx990803
Copy link
Member

This is intended. It doesn't affect visual output so there's no point to be "consistent" with 2.x.

@underfin
Copy link
Member

I track this with 2.x.Find some inconsistent behavior with the blew.It's has a bit of weird.

      // ""
      return this.$createElement("div", [true]);
      // "true"
      return this.$createElement("div", true);

@tangjinzhou
Copy link
Contributor Author

Too ugly, there will be many comment nodes on the DOM structure.

@yyx990803
Copy link
Member

Your user doesn't inspect your DOM.

@umairqazi523
Copy link

Your user doesn't inspect your DOM.

Some of them do.

@ariviom
Copy link

ariviom commented Jul 18, 2022

This issue is long closed now, but comment DOM nodes to affect CSS pseudo-selectors like :first and can cause unexpected behavior and hurdles when styling, even if users aren't inspecting the DOM.

@pangao66
Copy link

pangao66 commented Feb 14, 2023

here I have a case
when the dom is a empty dom, it show - ;
it often used in a table, when the cell is empty , show a hyphen
I can use empty css selectors to select the empty cell

.ant-table-cell{
  &:empty {
    &:after {
      content: '-';
    }
  }
}

but if the dom is empty content but threre are comment dom node by v-if, this empty css selector can not select it
even though there is a :blank css selector will be ok, but now the :blank css css selector is not support by any browser
我有这种场景, 当dom内容是空的时候,需要显示 一个短横线 连字符
尤其在table中, 当空单元格的时候,会经常碰到这种场景, 这时候可以使用 css empty选择器来选中这些空的单元格,但是
如果 单元格中是空内容但是有 v-if导致的注释元素, empty选择器就不能选中这些空单元格了,
现在有个:blank选择器提案,可以选中这种,但目前没有浏览器支持

@silverwind
Copy link

I find it rather concerning that vue gives a meaning to DOM comments. Comments can cause various issues:

  • Mechanisms like MutationObserver will see them, so will need to perform useless extra iterations.
  • For SSR, perservation of HTML comments can not be guaranteed as minifying proxies like Cloudflare's will strip them.

@Trobyss
Copy link

Trobyss commented Mar 31, 2023

Hi !

I asking if this comment doesn't affect DOM size when the navigator downloads the current page. In my case, on a complex page, I have more than 300 empty comments...

'Trob

@github-actions github-actions bot locked and limited conversation to collaborators Sep 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants