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.

Sign up for GitHub

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

Jump to bottom

Add support for rendering into non-body DOM nodes #249

Merged
merged 4 commits into from Dec 1, 2019
Merged

Add support for rendering into non-body DOM nodes #249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5066a6f
Add support for rendering into non-body DOM nodes
slimsag Dec 1, 2019
02970fb
fix tests
slimsag Dec 1, 2019
87242bb
fix tests / update test data
slimsag Dec 1, 2019
864c5b5
improve panic / error naming (use correct method name always)
slimsag Dec 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
92 changes: 76 additions & 16 deletions dom.go
View file
Open in desktop
@@ -1,6 +1,8 @@
package vecty

import "reflect"
import (
"reflect"
)

// batch renderer singleton
var batch = &batchRenderer{idx: make(map[Component]int)}
Expand Down Expand Up @@ -1155,45 +1157,102 @@ func requestAnimationFrame(callback func(float64)) int {
}

// RenderBody renders the given component as the document body. The given
// Component's Render method must return a "body" element.
// Component's Render method must return a "body" element or a panic will
// occur.
//
// This function blocks forever in order to prevent the program from exiting,
// which would prevent components from rerendering themselves in the future.
//
// It is a short-handed form for writing:
//
// err := vecty.RenderInto("body", body)
// if err !== nil {
// panic(err)
// }
// select{} // run Go forever
//
func RenderBody(body Component) {
target := global.Get("document").Call("querySelector", "body")
err := renderIntoNode("RenderBody", target, body)
if err != nil {
panic(err)
}
if !isTest {
select {} // run Go forever
}
}

// ElementMismatchError is returned when the element returned by a component
// does not match what is required for rendering.
type ElementMismatchError struct {
method, got, want string
}

func (e ElementMismatchError) Error() string {
return "vecty: " + e.method + `: expected Component.Render to return a "` + e.want + `", found "` + e.got + `"`
}

// InvalidTargetError is returned when the element targeted by a render is
// invalid because it is null or undefined.
type InvalidTargetError struct {
method string
}

func (e InvalidTargetError) Error() string {
return "vecty: " + e.method + `: invalid target element is null or undefined`
}

// RenderInto renders the given component into the existing HTML element found
// by the CSS selector (e.g. "#id", ".class-name") by replacing it.
//
// If there is more than one element found, the first is used. If no element is
// found, an error of type InvalidTargetError is returned.
//
// If the Component's Render method does not return an element of the same type,
// an error of type ElementMismatchError is returned.
func RenderInto(selector string, c Component) error {
target := global.Get("document").Call("querySelector", selector)
return renderIntoNode("RenderInto", target, c)
}

func renderIntoNode(methodName string, node jsObject, c Component) error {
if !node.Truthy() {
return InvalidTargetError{method: methodName}
}
// block batch until we're done
batch.scheduled = true
nextRender, skip, pendingMounts := renderComponent(body, nil)
nextRender, skip, pendingMounts := renderComponent(c, nil)
if skip {
panic("vecty: RenderBody Component.SkipRender illegally returned true")
panic("vecty: " + methodName + ": Component.SkipRender illegally returned true")
}
if nextRender.tag != "body" {
panic("vecty: RenderBody expected Component.Render to return a body tag, found \"" + nextRender.tag + "\"")
expectTag := node.Get("nodeName").String()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I'm running into an issue with nodeName case sensitivity here (see https://developer.mozilla.org/en-US/docs/Web/API/Node/nodeName and https://johnresig.com/blog/nodename-case-sensitivity/).

My program is doing:

// Initial frontend render.
err := vecty.RenderInto("body", body)
if err != nil {
	panic(fmt.Errorf("internal error: unexpected error from vecty.RenderInto: %v", err))
}

And getting:

Uncaught Error: internal error: unexpected error from vecty.RenderInto: vecty: RenderInto: expected Component.Render to return a "BODY", found "body"

I'm using HTML, Chrome 78.0.3904.108 on macOS.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried the following quick change to see if it would be sufficient to solve the problem:

diff --git a/dom.go b/dom.go
index 0eb000e..164a5a3 100644
--- a/dom.go
+++ b/dom.go
@@ -2,6 +2,7 @@ package vecty
 
 import (
 	"reflect"
+	"strings"
 )
 
 // batch renderer singleton
@@ -1225,7 +1226,7 @@ func renderIntoNode(methodName string, node jsObject, c Component) error {
 	if skip {
 		panic("vecty: " + methodName + ": Component.SkipRender illegally returned true")
 	}
-	expectTag := node.Get("nodeName").String()
+	expectTag := strings.ToLower(node.Get("nodeName").String())
 	if nextRender.tag != expectTag {
 		return ElementMismatchError{method: methodName, got: nextRender.tag, want: expectTag}
 	}

It was; after that, Go Package Store was rendering the frontend without issues.

I'm not sure if that's the optimal long term fix, or if there's a better way. You may not want to import a new strings package just for one function.

I tried using JavaScript's toLowerCase method, but for some reason that didn't work in the context of Vecty:

Uncaught Error: syscall/js: call of Value.Call on string

Even though it seems to work fine elsewhere, e.g., see https://gopherjs.github.io/playground/#/UxD9vDjbf0.

Hope this is helpful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About the "syscall/js: call of Value.Call on string" error, I found it happens when using syscall/js package with GopherJS but not when using github.com/gopherjs/gopherjs/js package. So it's not Vecty-specific beyond Vecty using syscall/js.

It happens when using syscall/js with both GopherJS and Go/WebAssembly, so I filed golang/go#35917 to learn more about that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for investigating this! I've sent #251 to fix the issue on the Vecty side

if nextRender.tag != expectTag {
return ElementMismatchError{method: methodName, got: nextRender.tag, want: expectTag}
}
doc := global.Get("document")
if doc.Get("readyState").String() == "loading" {
// avoid duplicate body
var cb jsFunc
cb = funcOf(func(this jsObject, args []jsObject) interface{} {
cb.Release()

doc.Set("body", nextRender.node)
replaceNode(nextRender.node, node)
mount(pendingMounts...)
if m, ok := body.(Mounter); ok {
if m, ok := c.(Mounter); ok {
mount(m)
}
requestAnimationFrame(batch.render)
return undefined
})
doc.Call("addEventListener", "DOMContentLoaded", cb)
return
return nil
}
doc.Set("body", nextRender.node)
replaceNode(nextRender.node, node)
mount(pendingMounts...)
if m, ok := body.(Mounter); ok {
if m, ok := c.(Mounter); ok {
mount(m)
}
requestAnimationFrame(batch.render)
if !isTest {
// run Go forever
select {}
}
return nil
}

// SetTitle sets the title of the document.
Expand All @@ -1219,6 +1278,7 @@ type jsObject interface {
Delete(key string)
Call(name string, args ...interface{}) jsObject
String() string
Truthy() bool
Bool() bool
Int() int
Float() float64
Expand Down
9 changes: 9 additions & 0 deletions dom_native.go
View file
Open in desktop
Expand Up @@ -28,6 +28,15 @@ func (h *HTML) Node() SyscallJSValue {
return htmlNodeImpl(h)
}

// RenderIntoNode renders the given component into the existing HTML element by
// replacing it.
//
// If the Component's Render method does not return an element of the same type,
// an error of type ElementMismatchError is returned.
func RenderIntoNode(node SyscallJSValue, c Component) error {
return renderIntoNode("RenderIntoNode", node, c)
}

var (
global jsObject
undefined wrappedObject
Expand Down
33 changes: 28 additions & 5 deletions dom_test.go
View file
Open in desktop
Expand Up @@ -563,6 +563,8 @@ func TestRerender_identical(t *testing.T) {

ts.ints.mock(`global.Call("requestAnimationFrame", func)`, 0)
ts.strings.mock(`global.Get("document").Get("readyState")`, "complete")
ts.strings.mock(`global.Get("document").Call("querySelector", "body").Get("nodeName")`, "body")
ts.truthies.mock(`global.Get("document").Call("querySelector", "body")`, true)

// Perform the initial render of the component.
render := Tag("body")
Expand Down Expand Up @@ -650,6 +652,8 @@ func TestRerender_change(t *testing.T) {

ts.ints.mock(`global.Call("requestAnimationFrame", func)`, 0)
ts.strings.mock(`global.Get("document").Get("readyState")`, "complete")
ts.strings.mock(`global.Get("document").Call("querySelector", "body").Get("nodeName")`, "body")
ts.truthies.mock(`global.Get("document").Call("querySelector", "body")`, true)

// Perform the initial render of the component.
render := Tag("body")
Expand Down Expand Up @@ -753,6 +757,8 @@ func TestRerender_Nested(t *testing.T) {

ts.ints.mock(`global.Call("requestAnimationFrame", func)`, 0)
ts.strings.mock(`global.Get("document").Get("readyState")`, "complete")
ts.strings.mock(`global.Get("document").Call("querySelector", "body").Get("nodeName")`, "body")
ts.truthies.mock(`global.Get("document").Call("querySelector", "body")`, true)

// Perform the initial render of the component.
var renderCalled, skipRenderCalled int
Expand Down Expand Up @@ -851,6 +857,8 @@ func TestRerender_persistent(t *testing.T) {

ts.ints.mock(`global.Call("requestAnimationFrame", func)`, 0)
ts.strings.mock(`global.Get("document").Get("readyState")`, "complete")
ts.strings.mock(`global.Get("document").Call("querySelector", "body").Get("nodeName")`, "body")
ts.truthies.mock(`global.Get("document").Call("querySelector", "body")`, true)

lastRenderedComponent = nil
renderCount = 0
Expand Down Expand Up @@ -913,6 +921,8 @@ func TestRerender_persistent_direct(t *testing.T) {

ts.ints.mock(`global.Call("requestAnimationFrame", func)`, 0)
ts.strings.mock(`global.Get("document").Get("readyState")`, "complete")
ts.strings.mock(`global.Get("document").Call("querySelector", "body").Get("nodeName")`, "body")
ts.truthies.mock(`global.Get("document").Call("querySelector", "body")`, true)

lastRenderedComponent = nil
renderCount = 0
Expand Down Expand Up @@ -959,24 +969,27 @@ func TestRenderBody_ExpectsBody(t *testing.T) {
{
name: "text",
render: Text("Hello world!"),
wantPanic: "vecty: RenderBody expected Component.Render to return a body tag, found \"\"", // TODO(slimsag): error message bug
wantPanic: "vecty: RenderBody: expected Component.Render to return a \"body\", found \"\"", // TODO(slimsag): error message bug
},
{
name: "div",
render: Tag("div"),
wantPanic: "vecty: RenderBody expected Component.Render to return a body tag, found \"div\"",
wantPanic: "vecty: RenderBody: expected Component.Render to return a \"body\", found \"div\"",
},
{
name: "nil",
render: nil,
wantPanic: "vecty: RenderBody expected Component.Render to return a body tag, found \"noscript\"",
wantPanic: "vecty: RenderBody: expected Component.Render to return a \"body\", found \"noscript\"",
},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
ts := testSuite(t)
defer ts.done()

ts.strings.mock(`global.Get("document").Call("querySelector", "body").Get("nodeName")`, "body")
ts.truthies.mock(`global.Get("document").Call("querySelector", "body")`, true)

var gotPanic string
func() {
defer func() {
Expand All @@ -993,7 +1006,7 @@ func TestRenderBody_ExpectsBody(t *testing.T) {
})
}()
if c.wantPanic != gotPanic {
t.Fatalf("want panic %q got panic %q", c.wantPanic, gotPanic)
t.Fatalf("want panic:\n%q\ngot panic:\n%q", c.wantPanic, gotPanic)
}
})
}
Expand All @@ -1005,6 +1018,8 @@ func TestRenderBody_RenderSkipper_Skip(t *testing.T) {
ts := testSuite(t)
defer ts.done()

ts.truthies.mock(`global.Get("document").Call("querySelector", "body")`, true)

comp := &componentFunc{
render: func() ComponentOrHTML {
return Tag("body")
Expand All @@ -1018,7 +1033,7 @@ func TestRenderBody_RenderSkipper_Skip(t *testing.T) {
got := recoverStr(func() {
RenderBody(comp)
})
want := "vecty: RenderBody Component.SkipRender illegally returned true"
want := "vecty: RenderBody: Component.SkipRender illegally returned true"
if got != want {
t.Fatalf("got panic %q want %q", got, want)
}
Expand All @@ -1033,6 +1048,8 @@ func TestRenderBody_Standard_loaded(t *testing.T) {

ts.strings.mock(`global.Get("document").Get("readyState")`, "loaded")
ts.ints.mock(`global.Call("requestAnimationFrame", func)`, 0)
ts.strings.mock(`global.Get("document").Call("querySelector", "body").Get("nodeName")`, "body")
ts.truthies.mock(`global.Get("document").Call("querySelector", "body")`, true)

RenderBody(&componentFunc{
render: func() ComponentOrHTML {
Expand All @@ -1050,6 +1067,8 @@ func TestRenderBody_Standard_loading(t *testing.T) {

ts.strings.mock(`global.Get("document").Get("readyState")`, "loading")
ts.ints.mock(`global.Call("requestAnimationFrame", func)`, 0)
ts.strings.mock(`global.Get("document").Call("querySelector", "body").Get("nodeName")`, "body")
ts.truthies.mock(`global.Get("document").Call("querySelector", "body")`, true)

RenderBody(&componentFunc{
render: func() ComponentOrHTML {
Expand All @@ -1069,6 +1088,8 @@ func TestRenderBody_Nested(t *testing.T) {

ts.strings.mock(`global.Get("document").Get("readyState")`, "complete")
ts.ints.mock(`global.Call("requestAnimationFrame", func)`, 0)
ts.strings.mock(`global.Get("document").Call("querySelector", "body").Get("nodeName")`, "body")
ts.truthies.mock(`global.Get("document").Call("querySelector", "body")`, true)

RenderBody(&componentFunc{
render: func() ComponentOrHTML {
Expand Down Expand Up @@ -1109,6 +1130,8 @@ func TestKeyedChild_DifferentType(t *testing.T) {

ts.ints.mock(`global.Call("requestAnimationFrame", func)`, 0)
ts.strings.mock(`global.Get("document").Get("readyState")`, "complete")
ts.strings.mock(`global.Get("document").Call("querySelector", "body").Get("nodeName")`, "body")
ts.truthies.mock(`global.Get("document").Call("querySelector", "body")`, true)

comp := &componentFunc{
render: func() ComponentOrHTML {
Expand Down
13 changes: 13 additions & 0 deletions dom_wasmjs_gopherjs.go
View file
Open in desktop
Expand Up @@ -21,6 +21,15 @@ func (h *HTML) Node() js.Value {
return h.node.(wrappedObject).j
}

// RenderIntoNode renders the given component into the existing HTML element by
// replacing it.
//
// If the Component's Render method does not return an element of the same type,
// an error of type ElementMismatchError is returned.
func RenderIntoNode(node js.Value, c Component) error {
return renderIntoNode("RenderIntoNode", wrapObject(node), c)
}

var (
global = wrapObject(js.Global())
undefined = wrappedObject{js.Undefined()}
Expand Down Expand Up @@ -103,6 +112,10 @@ func (w wrappedObject) String() string {
return w.j.String()
}

func (w wrappedObject) Truthy() bool {
return w.j.Truthy()
}

func (w wrappedObject) Bool() bool {
return w.j.Bool()
}
Expand Down
6 changes: 5 additions & 1 deletion testdata/TestKeyedChild_DifferentType.want.txt
View file
Open in desktop
@@ -1,4 +1,6 @@
global.Get("document")
global.Get("document").Call("querySelector", "body")
global.Get("document")
global.Get("document").Call("createElement", "body")
global.Get("document").Call("createElement", "body").Get("classList")
global.Get("document").Call("createElement", "body").Get("dataset")
Expand All @@ -9,9 +11,11 @@ global.Get("document").Call("createElement", "tag1").Get("classList")
global.Get("document").Call("createElement", "tag1").Get("dataset")
global.Get("document").Call("createElement", "tag1").Get("style")
global.Get("document").Call("createElement", "body").Call("appendChild", jsObject(global.Get("document").Call("createElement", "tag1")))
global.Get("document").Call("querySelector", "body").Get("nodeName")
global.Get("document")
global.Get("document").Get("readyState")
global.Get("document").Set("body", jsObject(global.Get("document").Call("createElement", "body")))
global.Get("document").Call("querySelector", "body").Get("parentNode")
global.Get("document").Call("querySelector", "body").Get("parentNode").Call("replaceChild", jsObject(global.Get("document").Call("createElement", "body")), jsObject(global.Get("document").Call("querySelector", "body")))
global.Call("requestAnimationFrame", func)
global.Get("document").Call("createElement", "body").Get("classList")
global.Get("document").Call("createElement", "body").Get("dataset")
Expand Down
5 changes: 4 additions & 1 deletion testdata/TestRenderBody_ExpectsBody__div.want.txt
View file
Open in desktop
@@ -1,5 +1,8 @@
global.Get("document")
global.Get("document").Call("querySelector", "body")
global.Get("document")
global.Get("document").Call("createElement", "div")
global.Get("document").Call("createElement", "div").Get("classList")
global.Get("document").Call("createElement", "div").Get("dataset")
global.Get("document").Call("createElement", "div").Get("style")
global.Get("document").Call("createElement", "div").Get("style")
global.Get("document").Call("querySelector", "body").Get("nodeName")
5 changes: 4 additions & 1 deletion testdata/TestRenderBody_ExpectsBody__nil.want.txt
View file
Open in desktop
@@ -1,5 +1,8 @@
global.Get("document")
global.Get("document").Call("querySelector", "body")
global.Get("document")
global.Get("document").Call("createElement", "noscript")
global.Get("document").Call("createElement", "noscript").Get("classList")
global.Get("document").Call("createElement", "noscript").Get("dataset")
global.Get("document").Call("createElement", "noscript").Get("style")
global.Get("document").Call("createElement", "noscript").Get("style")
global.Get("document").Call("querySelector", "body").Get("nodeName")
5 changes: 4 additions & 1 deletion testdata/TestRenderBody_ExpectsBody__text.want.txt
View file
Open in desktop
@@ -1,5 +1,8 @@
global.Get("document")
global.Get("document").Call("querySelector", "body")
global.Get("document")
global.Get("document").Call("createTextNode", "Hello world!")
global.Get("document").Call("createTextNode", "Hello world!").Get("classList")
global.Get("document").Call("createTextNode", "Hello world!").Get("dataset")
global.Get("document").Call("createTextNode", "Hello world!").Get("style")
global.Get("document").Call("createTextNode", "Hello world!").Get("style")
global.Get("document").Call("querySelector", "body").Get("nodeName")
6 changes: 5 additions & 1 deletion testdata/TestRenderBody_Nested.want.txt
View file
Open in desktop
@@ -1,9 +1,13 @@
global.Get("document")
global.Get("document").Call("querySelector", "body")
global.Get("document")
global.Get("document").Call("createElement", "body")
global.Get("document").Call("createElement", "body").Get("classList")
global.Get("document").Call("createElement", "body").Get("dataset")
global.Get("document").Call("createElement", "body").Get("style")
global.Get("document").Call("querySelector", "body").Get("nodeName")
global.Get("document")
global.Get("document").Get("readyState")
global.Get("document").Set("body", jsObject(global.Get("document").Call("createElement", "body")))
global.Get("document").Call("querySelector", "body").Get("parentNode")
global.Get("document").Call("querySelector", "body").Get("parentNode").Call("replaceChild", jsObject(global.Get("document").Call("createElement", "body")), jsObject(global.Get("document").Call("querySelector", "body")))
global.Call("requestAnimationFrame", func)
2 changes: 2 additions & 0 deletions testdata/TestRenderBody_RenderSkipper_Skip.want.txt
View file
Open in desktop
@@ -0,0 +1,2 @@
global.Get("document")
global.Get("document").Call("querySelector", "body")