albrow / vdom

A virtual dom implementation written in go which is compatible with gopherjs
92 stars 12 forks source link

Replace.Patch(): "HierarchyRequestError: Node cannot be inserted at the specified point in the hierarchy" #1

Closed dradtke closed 7 years ago

dradtke commented 7 years ago

I believe I found a bug in the implementation of Replace.Patch(). Given some GopherJS resembling this:

type State struct {
    User    *models.User // Contains Email and DisplayName fields
    Editing bool
}

// These callbacks made global via js.Global.Set() and MakeFunc()

func EditAccount(this *js.Object, arguments []*js.Object) interface{} {
    state.Editing = true
    render()
    return nil
}

func CancelEdit(this *js.Object, arguments []*js.Object) interface{} {
    state.Editing = false
    render()
    return nil
}

and this HTML template:

<table class="table">
  <tbody>
    <tr>
      <td><strong class="pull-right">Email:</strong></td>
      <td>{{.User.Email}}</td>
    </tr>
    <tr>
      <td><strong class="pull-right">Display Name:</strong></td>
      <td>
        {{if .Editing}}<input type="text" value="{{.User.DisplayName}}">{{else}}<span>{{.User.DisplayName}}</span>{{end}}
      </td>
    </tr>
  </tbody>
</table>

<!-- Event listeners made global via js.Global.Set() and MakeFunc() -->
<p class="text-center">
  {{if .Editing}}
    <button type="button" class="btn btn-default" onclick="CancelEdit()">Cancel</button>
  {{else}}
    <button type="button" class="btn btn-primary" onclick="EditAccount()">Edit</button>
  {{end}}
</p>

Then the initial render works fine, but clicking on the Edit button results in this error appearing in the Javascript console:

HierarchyRequestError: Node cannot be inserted at the specified point in the hierarchy

After some investigation, I came up with a fix that I believe is a bug in the Replace.Patch() method. Below are the original implementation and my new one:

// Original
func (p *Replace) Patch(root dom.Element) error {
    var parent dom.Node
    if p.Old.Parent() != nil {
        parent = findInDOM(p.Old.Parent(), root)
    } else {
        parent = root
    }
    oldChild := findInDOM(p.Old, root)
    newChild := createForDOM(p.New)
    parent.ReplaceChild(newChild, oldChild)
    return nil
}

// Revised
func (p *Replace) Patch(root dom.Element) error {
    oldChild := findInDOM(p.Old, root)
    newChild := createForDOM(p.New)
    oldChild.ParentNode().ReplaceChild(newChild, oldChild)
    return nil
}

With the revised version of the method, everything works as expected. The call to findInDOM(p.Old.Parent(), root) is apparently returning an incorrect result. One alternative fix is to change that line to be findInDOM(p.Old, root).ParentNode(), with my revised version just being an optimization of that.

dradtke commented 7 years ago

Superseded by https://github.com/albrow/vdom/pull/2.