Hello World Patch Error

norunners commented 6 years ago

Hello, I've encountered an error while calling vdom.Patch on the second render, the first render is successful. Am I missing something critical in this example or is this a bug?

Error: Error: runtime error: index out of range

hello.js:1412:17
$callDeferred
http://localhost:8080/github.com/albrow/vdom/examples/hello/hello.js:1412:17
$panic
http://localhost:8080/github.com/albrow/vdom/examples/hello/hello.js:1451:3
throw$1
runtime.go:221:2
findInDOM
http://localhost:8080/github.com/albrow/vdom/examples/hello/hello.js:37669:54
Replace.ptr.prototype.Patch
patch.go:84:2
PatchSet.prototype.Patch
patch.go:34:6
Hello.ptr.prototype.Render
main.go:38:5
$b
main.go:55:3
$goroutine
http://localhost:8080/github.com/albrow/vdom/examples/hello/hello.js:1471:15
$runScheduled
http://localhost:8080/github.com/albrow/vdom/examples/hello/hello.js:1511:7
$schedule
http://localhost:8080/github.com/albrow/vdom/examples/hello/hello.js:1527:5
$go
http://localhost:8080/github.com/albrow/vdom/examples/hello/hello.js:1503:3
startTimer/t.timeout<
time.go:71:3
$setTimeout/<
http://localhost:8080/github.com/albrow/vdom/examples/hello/hello.js:1535:5

Source: main.go

package main

import (
    "bytes"
    "github.com/albrow/vdom"
    "honnef.co/go/js/dom"
    "html/template"
    "time"
)

const tmpl = `<p>{{ .Message }}</p>`

type Hello struct {
    Message string

    tmpl *template.Template
    root dom.Element
    tree *vdom.Tree
}

func (hello *Hello) Render() error {
    // Execute the template with the given todo and write to a buffer
    buf := bytes.NewBuffer(nil)
    if err := hello.tmpl.Execute(buf, hello); err != nil {
        return err
    }
    // Parse the resulting html into a virtual tree
    newTree, err := vdom.Parse(buf.Bytes())
    if err != nil {
        return err
    }
    // Calculate the diff between this render and the last render
    patches, err := vdom.Diff(hello.tree, newTree)
    if err != nil {
        return err
    }
    // Efficiently apply changes to the actual DOM
    if err := patches.Patch(hello.root); err != nil {
        return err
    }
    // Remember the virtual DOM state for the next render to diff against
    hello.tree = newTree
    return nil
}

func main() {
    root := dom.GetWindow().Document().GetElementByID("app")
    tmpl := template.Must(template.New("hello").Parse(tmpl))
    hello := &Hello{Message: "Hello", tmpl: tmpl, root: root, tree: &vdom.Tree{}}
    err := hello.Render()
    must(err)

    time.AfterFunc(time.Second, func() {
        hello.Message = "World!"
        err = hello.Render()
        must(err)
    })
}

func must(err error) {
    if err != nil {
        panic(err)
    }
}

Source: index.html

<!doctype html>
<html>
    <head>
        <meta charset="utf-8">
        <title>Go vdom</title>
    </head>
    <body>
        <div id="app">
        </div>
        <script src="hello.js"></script>
    </body>
</html>

First render:

<!DOCTYPE html>
<html>
    <head>
        <meta charset="utf-8">
        <title>Go vdom</title>
    </head>
    <body>
        <div id="app">
            <p>Hello</p>
        </div>
        <script src="hello.js"></script>
    </body>
</html>

albrow commented 6 years ago

@norunners thank you for reporting this. I can't promise that I will be able to fix it anytime soon as vdom has not been a priority for me for a long time now. Still, I may take a look if I have time.

If you want to do more digging yourself, I would start by taking a closer look at the findInDom function as I think the issue is originating there.

dradtke commented 6 years ago

Hey @norunners, I think I discovered a workaround, though it doesn't entirely fix the issue. I was able to get it to work by changing index.html from this:

<div id="app">
</div>

to this:

<div id="app"></div>

I'm probably going to do a little more digging, but that should help get you up and running at least.

dradtke commented 6 years ago

I've discovered a little more about the root cause, but haven't developed a proper fix yet. It comes down to the way that HTML nodes are represented in the DOM, and some assumptions that are apparently made by vdom's diffing algorithm.

Ultimately, the issue is that in the following two examples, the app div has two different child node counts:

<!-- app has only one child: the paragraph element -->
<div id="app"><p>Hello!</p></div>

<!-- app has two children: a text node, and the paragraph element -->
<div id="app"> <p>Hello!</p></div>

vdom uses index lists to traverse from a root element to the one that needs to be modified, and in both these cases, it attempts to navigate to the Hello! text node using the list [0 0]. In the second case, it stumbles upon the text node, which has no children and therefore causes a panic.

dradtke commented 6 years ago

It looks like the root cause is a fundamental difference in how XML and HTML are parsed. Currently, vdom uses an XML parser tuned with some recommended settings for parsing (most) HTML. However, if you look at the documentation for "encoding/xml".Unmarshal, it points out that whitespace is always trimmed and ignored. This is a subtle but important distinction between how the parser works and how the underlying dom package works, which treats whitespace as significant.

The correct fix for this is to update vdom to use an HTML parser instead of XML. The good news is that Go has one that is officially maintained, albeit not technically in the standard library. The bad news is that it's quite a large change to switch out the underlying parser.

Investigating this has also given me the opportunity to re-evaluate vdom's API, and propose a way to potentially make it simpler. I started developing a proof-of-concept in a new repository: https://github.com/dradtke/vdom2. It's extremely simple and would need a lot more work, but does work for norunner's original use-case (see that repository's example folder), and avoids the bug that originally prompted this issue.

norunners commented 6 years ago

Thank you for investigating this and providing detailed explanations. I’m going to try and use an html minifier before parsing as a short term fix. I’ll check out vdom2 and stay tuned in the future. Also, does it make sense to have a patch for event listeners? Maybe this need not be a concern of a virtue dom.

pyrossh commented 5 years ago

Nice work @dradtke what do you think the vdom implementation of vecty? https://github.com/gopherjs/vecty/blob/master/dom.go

I'm also creating a vdom framework like vecty here, https://github.com/krab-dev/exo/blob/master/html.go

albrow / vdom

Hello World Patch Error #3