Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

Hello World Patch Error #3

Open
norunners opened this issue Jul 31, 2018 · 6 comments
Open

Hello World Patch Error #3

norunners opened this issue Jul 31, 2018 · 6 comments
Assignees
Labels

Comments

@norunners
Copy link

norunners commented Jul 31, 2018

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>
@norunners norunners changed the title Hello World Parse Error Hello World Patch Error Jul 31, 2018
@albrow albrow self-assigned this Aug 3, 2018
@albrow albrow added the bug label Aug 3, 2018
@albrow
Copy link
Owner

albrow commented Aug 3, 2018

@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
Copy link
Contributor

dradtke commented Aug 17, 2018

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
Copy link
Contributor

dradtke commented Aug 17, 2018

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
Copy link
Contributor

dradtke commented Aug 17, 2018

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
Copy link
Author

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
Copy link

pyrossh commented Aug 23, 2019

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants