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

The insert method get ref.parentNode at patch.js, it should be nodeOps.parentNode(ref) #8713

Closed
akira-cn opened this issue Aug 26, 2018 · 0 comments
Labels

Comments

@akira-cn
Copy link

akira-cn commented Aug 26, 2018

Version

2.5.17

Reproduction link

https://code.h5jun.com/mewa/edit?html,output

Steps to reproduce

Note: Reproduction link 里面的bug我是已经临时修了,用这个文件覆盖了patch.js

我在修改runtime以支持spritejs,发现vdom的patch.js里面使用了ref.parentNode

因为v-if指令用CommentNode作为占位符,但是spritejs是基于canvas的,没有实体DOM,所以我没办法把CommentNode添加到DOM上,我做了一个适配,将它作为spritejs的children添加到容器里。这么做我需要改变该占位元素的parent,因为这是一个HTML元素,parentNode是只读的,我没法直接修改,所以我在这个元素上添加一个新的parent属性:

export function createComment (text: string): Comment {
  const comment = document.createComment(text)
  comment.connect = (parent, zOrder) => {
    comment.parent = parent
    comment.zOrder = zOrder
  }
  comment.disconnect = (parent) => {
    delete comment.parent
    delete comment.zOrder
  }
  comment.dispatchEvent = () => false
  comment.forceUpdate = () => false
  comment.isVisible = () => false
  return comment
}

然后重写:

export function parentNode (node: Node): ?Node {
  return node.parentNode || node.parent
}

结果发现使用 v-if 切换的时候不行(现象是toggle元素的时候能隐藏不能显现)。后来我发现在patch.js里这段代码:

  function insert (parent, elm, ref) {
    if (isDef(parent)) {
      if (isDef(ref)) {
        if (ref.parentNode === parent) {
          nodeOps.insertBefore(parent, elm, ref)
        }
      } else {
        nodeOps.appendChild(parent, elm)
      }
    }
  }

应该改为:

  function insert (parent, elm, ref) {
    if (isDef(parent)) {
      if (isDef(ref)) {
        if (nodeOps.parentNode(ref) === parent) {
          nodeOps.insertBefore(parent, elm, ref)
        }
      } else {
        nodeOps.appendChild(parent, elm)
      }
    }
  }

这样才能拿到正确的parentNode。

因为只有一行代码修改,我就不提PR了。我现在先采用patch-fix的方式修复。

如果下一个版本能顺便修一下,我会很感激 :)。

What is expected?

应该得到:nodeOps.parentNode(ref)

What is actually happening?

实际得到:ref.parentNode

@akira-cn akira-cn changed the title patch 的 insert 方法硬编码了判断 ref.parentNode 的逻辑 The insert method get ref.parentNode at patch.js, it should be nodeOps.parentNode(ref) Aug 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants