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

Allow Render() return nil #689

Open
malumar opened this issue Jan 20, 2022 · 9 comments
Open

Allow Render() return nil #689

malumar opened this issue Jan 20, 2022 · 9 comments

Comments

@malumar
Copy link

malumar commented Jan 20, 2022

At present, the component must render any content. If NIL is returned, the rendering engine should ignore the value and move on to the next element.

Currently, the component must render any content. If NIL is returned, the rendering engine should ignore the value and move on to the next element. Without this functionality, you cannot create a complete virtual component that does not draw any state when needed.

@pedronasser
Copy link

This is a very good feature. I am tired of having to return an empty div.
Is there any alternative currently?

@oderwat
Copy link
Sponsor Contributor

oderwat commented Feb 8, 2023

I am curious: Why would one have a component without any content? My root component does contain another component because I have a central navigation handler for all embedded components. But without this content, I don't see what I could do with a "nil" component.

@pedronasser
Copy link

pedronasser commented Feb 15, 2023

@oderwat Sometimes, depending on the state/props of the component, you want the component to not return anything.
In React this is a very common and useful pattern.

@oderwat
Copy link
Sponsor Contributor

oderwat commented Feb 15, 2023

Nope, I have not and avoided that experience successfully so far. I still wonder how that relates to go-app, where a component is something that has a DOM node.

@pedronasser
Copy link

@oderwat For example

type MyComponent struct {
	app.Compo

	isLoggedIn bool
}

func (c *MyComponent) OnMount(ctx app.Context) {
	ctx.ObserveState("isLoggedIn").Value(&c.isLoggedIn)
}

func (c *MyComponent) Render() app.UI {
	return app.Div().Body(
		app.If(c.isLoggedIn,
			app.H1().Body(app.Text("You are logged in!")),
		),
	)
}

If the isLoggedIn is false, the component will still render an empty div, correct?
Let's say for example you have this component inside a Stack component (basically a div, with a flex box direction column, and probably a gap between the children).
With this current implementation, you cant prevent that component of rendering anything unless you have the same condition on the parent to prevent the component to be rendered.

So basically you can't have something like this, can you?

...
func (p *Parent) Render() app.UI {
	return VerticalStack().Body(
		&MyComponent{},
		&MyOtherComponent{}, 
	)
}
...

@oderwat
Copy link
Sponsor Contributor

oderwat commented Feb 15, 2023

So basically you can't have something like this, can you?

Correct. But I am very fine with that because I do not want that component in the first place. Or better: I do not know what should be "mounted" in that case. You simply have nothing to mount. One could argue that the component has to "decide" if it needs to be rendered, and then I would simply hide the (div) node. I also used .container { display: contents; } in similar situations.

What you want is that the component is not mounted and decide that after / in the OnMount() but then it is too late, as the node was created before. Render() will be called before OnMount(), so it would need to "not really create and mount the node" and therefore would not be mounted in OnMount() and that would be strange. Even if you had this, it would need to update the component when you change it's state, and then it would call OnMount() again because it was not mounted before. Basically: You can't have a component that is "mounted" but not there, if "mounted" means that it is part of the DOM.

P.S.: Using "app.If()" for more than just an example would be forbidden in our team. I still think that this function needs to be deprecated (because there is no lazy evaluation and this is not obvious and causes resource usage for "nothing").

@pedronasser
Copy link

pedronasser commented Feb 16, 2023

@oderwat I understand your perspective.

But even though this package is not meant for React projects, it still builds applications using the DOM. From my experience working with frameworks that use the DOM, having the ability to control whether a component is mounted or unmounted is crucial for large projects. Without this control, it is unlikely that this package will be adopted for big and complex projects.

While there is a cost and delay for WASM to manipulate the DOM, I believe this issue needs to be addressed in some way. Maybe temporarily moving the component out of the document while it is unmounted could be a solution, but I'm not sure how practical that is right now.

I understand that go-app may not be designed for complex applications, but having this kind of control is still useful and valid for larger projects.

@oderwat
Copy link
Sponsor Contributor

oderwat commented Feb 16, 2023

Well, I did not have any problem in our own larger projects yet and don't expect them. You have a point that compossibility suffers without having a nullable component, but I am not convinced that this makes complex applications impossible.

@maxence-charriere what do you think about this topic and could render nil be (easily) supported? I think it would "mount" but the JS value of the component would be nil in that case and everything in the DOM would be the same as if it would actually not being mounted. This could also be useful to add non DOM related functionality in a composable way using "null components".

@maxence-charriere
Copy link
Owner

I ll check what can be done.

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

No branches or pull requests

4 participants