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

explain why child renders cannot be equal #217

Open
progrium opened this issue Oct 29, 2018 · 6 comments
Open

explain why child renders cannot be equal #217

progrium opened this issue Oct 29, 2018 · 6 comments
Labels
Milestone

Comments

@progrium
Copy link

progrium commented Oct 29, 2018

I've been adding slots to my template system for Vecty so you can do this:

type Example struct {
	vecty.Core
	Children vecty.List `vecty:"slot"`
}

used like this:

<Example>
  <child></child>
  <child></child>
</Example>

where Example might have a template like this:

<div class="example">
  <slot></slot>
</div>

The result would be:

<div class="example">
  <child></child>
  <child></child>
</div>

And this works fine without really any changes to Vecty which is great, HOWEVER when I do rerenders of Example I run into this error:
https://github.com/gopherjs/vecty/blob/master/dom.go#L508-L510

And I've tried doing shallow copies of Children elements and I'm sure I'm just not knowing how it all works well enough to do this right, but the only way I can get it to work so far is to remove that check. And everything seems to work fine. So I'm wondering why it's there? This might also help me figure out how to do this the right way.

@pdf
Copy link
Contributor

pdf commented Oct 29, 2018

That's certainly a fair question @progrium - take a browse through #78 and all the associated issues. Short answer is: some time ago, allowing this caused for some subtle and non-obvious breakages, however that's suspected to have bene fixed, but not adequately tested to be certain.

@progrium
Copy link
Author

Huh. Quite a while ago. I guess you were running into it as well, what are you doing in the meantime?

@pdf
Copy link
Contributor

pdf commented Oct 30, 2018

My primary case was around reusing components rather than HTML, but we did a whole lot of work after I lodged that, so the renderer has changed significantly since then. Components are now reusable, but that panic persists for HTML, mainly because we've not had enough time to adequately test that it is safe to remove it now (I know I've personally had little time in the past year or so to contribute, and I suspect it has been similarly difficult for slimsag). As I mentioned in #124 (comment) I think it may be safe to remove now though.

@progrium
Copy link
Author

I mean, I can write tests. I just need to know what I'm looking for. I don't understand the failure cases it's there for. Honestly, if we can't articulate it, I'd say remove it until we can. And then fix/write tests for that.

@pdf
Copy link
Contributor

pdf commented Oct 30, 2018

The original description is in #70 but it's not the easiest conversation to parse at this remove.

Dredging my memory of about a year ago, I think what we're concerned with here is that if an instance of *HTML persists between renders somehow, that we correctly render all modifications to that instance and its children in all standard render scenarios. This would include all rendered properties/attributes/children/event-handlers/etc, and scenarios like: being at the root of a component render; nested under the root at x,y,z levels of nesting, of both Components and HTML; as a vecty.List member; when reordered via keys; when replacing one element with another; etc. I unfortunately don't have an exhaustive list off the top of my head though, and I'd have to spend some time reacquainting myself with the code to be certain exactly what's required.

If there are bugs that would be exercised by hitting this condition without triggering the panic, I suspect they will exhibit as elements not updating their rendered output as expected during rerender.

If my recollections are incorrect, hopefully slimsag will chime in to correct me.

@Gelio
Copy link

Gelio commented Dec 12, 2020

One workaround I found for this exact check is to accept a function that returns vecty.ComponentOrHTML instead of just the component itself. Then, in the Render function of the component, call that function to produce new children.

Example:

type Box struct {
	vecty.Core
	Children func() vecty.ComponentOrHTML `vecty:"slot"`
}

func (b *Box) Render() vecty.ComponentOrHTML {
	return elem.Div(
		vecty.Markup(
			vecty.Style("position", "relative"),
			vecty.Style("background-color", "#ffc0c0"),
			vecty.Style("width", "100%"),
			vecty.Style("height", "100%"),
		),
		elem.Div(
			vecty.Text("Hello world"),
			b.Children(),
		),
	)
}

As far as I have seen, this persists the state of the children component, so new structs can safely be constructed upon each rerender.

See https://github.com/Gelio/go-js-diagram/blob/02918eb210f204a8b5db3fea7f8d28792581480c/pkg/components/box.go#L19 for a more real-life example (a draggable box component)

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

4 participants