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

feature: add templ.HasChildren function #975

Open
epicbytes opened this issue Oct 30, 2024 · 11 comments
Open

feature: add templ.HasChildren function #975

epicbytes opened this issue Oct 30, 2024 · 11 comments
Labels
enhancement New feature or request NeedsInvestigation Issue needs some investigation before being fixed

Comments

@epicbytes
Copy link

Some components should have a default display if no child is passed, but the problem is that when callinggetChildren(ctx).
But I get NopComponent. This is unacceptable because there is a nil check in the same function. I can't compare and understand whether there are child components or not.

Please add the HasChildren(ctx) function to the release, which will give an understanding.

So, i want to do like this one:

if children == nil {
	<input { *ic.Attributes... }/>
} else {
	{ children... }
}
@a-h
Copy link
Owner

a-h commented Oct 31, 2024

Good idea.

I propose a design like this:

func Children(ctx context.Context) (ok bool, c templ.Component) {
	_, v := getContext(ctx)
	if v.children == nil {
		return false, NopComponent
	}
	return true, *v.children
}  

So you could use it as:

if children, ok := templ.Children(ctx); ok {
  @children
} else {
  <div>No children</div>
}

This wouldn't replace the { children... } syntax, but { children...} is already redundant, since you can do @templ.GetChildren(ctx) to get the same result. It would be possible to update templ to replace { children...} with a @templ.GetChildren(ctx) statement if we wanted to slim down, but it's out of scope for this.

To implement this, we'd need some generator tests to ensure that if, if / else behaviour works OK, and that you get a NopComponent instead of nil if there are no children to prevent accidental panics where the ok value is ignored or used incorrectly. We'd also need some unit tests for the runtime function.

Happy to take a PR for this if someone else wants to implement, also happy to hear comments on the proposal, including naming the function something else.

@epicbytes
Copy link
Author

Acceptable, it looks like due to the early calculation of child components it would be logical to use a flag whether the child components were initially passed or not, because the order of the call would then matter.

@a-h
Copy link
Owner

a-h commented Nov 9, 2024

I'm not sure I completely follow. Are you suggesting a different API?

We have an experimental package at https://github.com/templ-go/x - that might be a good place for a proof-of-concept design.

@a-h a-h changed the title Feature request: HasChildren feature request: templ.HasChildren function Nov 9, 2024
@a-h a-h changed the title feature request: templ.HasChildren function feature: add templ.HasChildren function Nov 9, 2024
@a-h a-h added enhancement New feature or request NeedsInvestigation Issue needs some investigation before being fixed labels Nov 9, 2024
@pulsone21
Copy link

How would you implement that in a templ List(){} like annotation?

I heavily using this kind of structure as the docs are telling, but I didn't find anything on how to get from a code component to a templ component.

@joerdav
Copy link
Collaborator

joerdav commented Dec 24, 2024

@pulsone21 it would be used as follows:

templ List() {
  if children, ok := templ.Children(ctx); ok {
    <ul>
      @children
    </ul>
  } else {
    <div>No children</div>
  }
}

templ Page() {
  @List() {
    <li>A</li>
    <li>B</li>
    <li>C</li>
  }
}

@pulsone21
Copy link

pulsone21 commented Dec 25, 2024

I have implemented the suggested change into my fork, see here:
main...pulsone21:templ:feature/Issue-975

The unit Tests are working, run go run . -r TestHasChildren

However i have the issue that the generator test test-has-children is failing.
The unit test works with templ.WithChildren(), but if i use it like @joerdav suggested it isn't working.

I struggle a bit to reverse engineer how this actually works, so i come to ask for some help.
Anyone can help?

PS: Prior a eventual PR to the main i would squash everything and clean it up.

@epicbytes
Copy link
Author

Yes, this is exactly what I mentioned at the beginning. The GetChildren function will alter the state of the context.

For example, I made a component like this:

templ HasChildTest() {
    if templ.HasChildren(ctx) {
        { children... }
    } else {
        <div> default component </div>
    }
}

I modified the templ source to test my theory:

func WithChildren(ctx context.Context, children Component) context.Context {
	ctx, v := getContext(ctx)
	v.children = &children
	return ctx
}

func ClearChildren(ctx context.Context) context.Context {
	_, v := getContext(ctx)
	v.children = nil
	return ctx
}

func HasChildren(ctx context.Context) bool {
	_, v := getContext(ctx)
	return v.children != nil
}

Here's the issue: after checking, the context is cleared of child documents, as seen in the generated code example:

@HasChildTest()

@HasChildTest() {
    <div>hello, im new child</div>
}
ctx = templ.InitializeContext(ctx)
	templ_7745c5c3_Var1 := templ.GetChildren(ctx)
	if templ_7745c5c3_Var1 == nil {
		templ_7745c5c3_Var1 = templ.NopComponent
	}
	ctx = templ.ClearChildren(ctx)
	_, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString("<section class=\"space-y-4 p-4\">")
	if templ_7745c5c3_Err != nil {
		return templ_7745c5c3_Err
	}
	templ_7745c5c3_Err = HasChildTest().Render(ctx, templ_7745c5c3_Buffer)
image

Yes, it seems that simply adding a HasChildren function in runtime won't solve the problem. The context state is being altered during the check and subsequent clear operation, which affects how components are rendered based on whether they have children or not.

@pulsone21
Copy link

pulsone21 commented Dec 26, 2024

Seems like a bug to me. I wouldn't expect the context is cleared after I get a value out of it.

Since I see now the clear children call I wonder if this is why the in initial proposal returned also the children's not only a book.
However if you look into the commit history of my fork you see that my initial implementation was like @a-h was suggesting.
But this wasn't working either, also it's hard to test if you can't compare two components.

@epicbytes
Copy link
Author

I have a proposal that I previously voiced - freezing the flag in terms of its initial state regarding the child elements. We are effectively freezing the variable storing the number of elements (someone might need to know this number), bypassing the cleanup of the child elements. I understand why cleaning is done so as not to confuse nested components, but it creates a problem with further work. Since code generation exists, why not use its capabilities?

@epicbytes
Copy link
Author

The information about child documents is being lost from the context before an attempt to retrieve it at the code level because code generation creates a precedent for clearing information about the presence of child elements.

@pulsone21
Copy link

pulsone21 commented Dec 26, 2024

I just updated the generator.go that it writes the ctx := templ.ClearChildren(ctx) after the children.
That made it work. See the commits here: main...pulsone21:templ:feature/Issue-975
image

The final template code would look like this:
generator/test-has-children/template_templ.go

		ctx = templ.InitializeContext(ctx)
		templ_7745c5c3_Var1 := templ.GetChildren(ctx)
		if templ_7745c5c3_Var1 == nil {
			templ_7745c5c3_Var1 = templ.NopComponent
		}
		if ok := templ.HasChildren(ctx); ok {
			_, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString("<ul>")
			if templ_7745c5c3_Err != nil {
				return templ_7745c5c3_Err
			}
			templ_7745c5c3_Err = templ_7745c5c3_Var1.Render(ctx, templ_7745c5c3_Buffer)
			if templ_7745c5c3_Err != nil {
				return templ_7745c5c3_Err
			}
			_, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString("</ul>")
			if templ_7745c5c3_Err != nil {
				return templ_7745c5c3_Err
			}
		} else {
			_, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString("<p>")
			if templ_7745c5c3_Err != nil {
				return templ_7745c5c3_Err
			}
			var templ_7745c5c3_Var2 string
			templ_7745c5c3_Var2, templ_7745c5c3_Err = templ.JoinStringErrs("No children")
			if templ_7745c5c3_Err != nil {
				return templ.Error{Err: templ_7745c5c3_Err, FileName: `generator/test-has-children/template.templ`, Line: 9, Col: 20}
			}
			_, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var2))
			if templ_7745c5c3_Err != nil {
				return templ_7745c5c3_Err
			}
			_, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString("</p>")
			if templ_7745c5c3_Err != nil {
				return templ_7745c5c3_Err
			}
		}
		ctx = templ.ClearChildren(ctx)
		return templ_7745c5c3_Err

In my local testing it worked so far:

go test ./...
ok  	github.com/a-h/templ	(cached)
?   	github.com/a-h/templ/cfg	[no test files]
ok  	github.com/a-h/templ/benchmarks/templ	(cached) [no tests to run]
ok  	github.com/a-h/templ/cmd/templ	(cached)
ok  	github.com/a-h/templ/cmd/templ/fmtcmd	(cached)
ok  	github.com/a-h/templ/cmd/templ/generatecmd	(cached)
ok  	github.com/a-h/templ/cmd/templ/generatecmd/modcheck	(cached)
?   	github.com/a-h/templ/cmd/templ/generatecmd/run/testprogram	[no test files]
?   	github.com/a-h/templ/cmd/templ/generatecmd/sse	[no test files]
ok  	github.com/a-h/templ/cmd/templ/generatecmd/proxy	(cached)
ok  	github.com/a-h/templ/cmd/templ/generatecmd/run	(cached)
ok  	github.com/a-h/templ/cmd/templ/generatecmd/symlink	(cached)
ok  	github.com/a-h/templ/cmd/templ/generatecmd/test-eventhandler	(cached)
ok  	github.com/a-h/templ/cmd/templ/generatecmd/testwatch	(cached)
ok  	github.com/a-h/templ/cmd/templ/generatecmd/watcher	(cached)
?   	github.com/a-h/templ/cmd/templ/infocmd	[no test files]
?   	github.com/a-h/templ/cmd/templ/lspcmd/lspdiff	[no test files]
?   	github.com/a-h/templ/cmd/templ/lspcmd/httpdebug	[no test files]
?   	github.com/a-h/templ/cmd/templ/lspcmd/pls	[no test files]
?   	github.com/a-h/templ/cmd/templ/sloghandler	[no test files]
?   	github.com/a-h/templ/cmd/templ/testproject	[no test files]
?   	github.com/a-h/templ/cmd/templ/visualize	[no test files]
?   	github.com/a-h/templ/examples/content-security-policy	[no test files]
?   	github.com/a-h/templ/examples/hello-world-ssr	[no test files]
?   	github.com/a-h/templ/examples/hello-world-static	[no test files]
?   	github.com/a-h/templ/generator/htmldiff	[no test files]
?   	github.com/a-h/templ/examples/syntax-and-usage/components	[no test files]
?   	github.com/a-h/templ/get-version	[no test files]
?   	github.com/a-h/templ/storybook	[no test files]
ok  	github.com/a-h/templ/cmd/templ/imports	3.368s
ok  	github.com/a-h/templ/cmd/templ/lspcmd	(cached)
ok  	github.com/a-h/templ/cmd/templ/lspcmd/proxy	(cached)
ok  	github.com/a-h/templ/cmd/templ/processor	(cached)
ok  	github.com/a-h/templ/examples/blog	(cached)
ok  	github.com/a-h/templ/generator	(cached)
ok  	github.com/a-h/templ/generator/test-a-href	(cached)
ok  	github.com/a-h/templ/generator/test-attribute-errors	(cached)
ok  	github.com/a-h/templ/generator/test-attribute-escaping	(cached)
ok  	github.com/a-h/templ/generator/test-call	(cached)
ok  	github.com/a-h/templ/generator/test-cancelled-context	(cached)
ok  	github.com/a-h/templ/generator/test-complex-attributes	(cached)
ok  	github.com/a-h/templ/generator/test-constant-attribute-escaping	(cached)
ok  	github.com/a-h/templ/generator/test-context	(cached)
ok  	github.com/a-h/templ/generator/test-css-expression	(cached)
ok  	github.com/a-h/templ/generator/test-css-middleware	(cached)
ok  	github.com/a-h/templ/generator/test-css-usage	(cached)
ok  	github.com/a-h/templ/generator/test-doctype	(cached)
ok  	github.com/a-h/templ/generator/test-element-attributes	(cached)
ok  	github.com/a-h/templ/generator/test-elseif	(cached)
ok  	github.com/a-h/templ/generator/test-for	(cached)
ok  	github.com/a-h/templ/generator/test-form-action	(cached)
ok  	github.com/a-h/templ/generator/test-go-comments	(cached)
ok  	github.com/a-h/templ/generator/test-go-template-in-templ	(cached)
ok  	github.com/a-h/templ/generator/test-has-children	(cached)
ok  	github.com/a-h/templ/generator/test-html	(cached)
ok  	github.com/a-h/templ/generator/test-html-comment	(cached)
ok  	github.com/a-h/templ/generator/test-if	(cached)
ok  	github.com/a-h/templ/generator/test-ifelse	(cached)
ok  	github.com/a-h/templ/generator/test-import	(cached)
ok  	github.com/a-h/templ/generator/test-method	(cached)
ok  	github.com/a-h/templ/generator/test-once	(cached)
ok  	github.com/a-h/templ/generator/test-only-scripts	(cached)
ok  	github.com/a-h/templ/generator/test-raw-elements	(cached)
ok  	github.com/a-h/templ/generator/test-script-inline	(cached)
ok  	github.com/a-h/templ/generator/test-script-usage	(cached)
ok  	github.com/a-h/templ/generator/test-script-usage-nonce	(cached)
ok  	github.com/a-h/templ/generator/test-spread-attributes	(cached)
ok  	github.com/a-h/templ/generator/test-string	(cached)
ok  	github.com/a-h/templ/generator/test-string-errors	(cached)
ok  	github.com/a-h/templ/generator/test-switch	(cached)
ok  	github.com/a-h/templ/generator/test-switchdefault	(cached)
ok  	github.com/a-h/templ/generator/test-templ-element	(cached)
ok  	github.com/a-h/templ/generator/test-templ-in-go-template	(cached)
ok  	github.com/a-h/templ/generator/test-text	(cached)
ok  	github.com/a-h/templ/generator/test-text-whitespace	(cached)
ok  	github.com/a-h/templ/generator/test-void	(cached)
ok  	github.com/a-h/templ/generator/test-whitespace-around-go-keywords	(cached)
ok  	github.com/a-h/templ/parser/v2	(cached)
ok  	github.com/a-h/templ/parser/v2/goexpression	(cached)
ok  	github.com/a-h/templ/runtime	(cached)
ok  	github.com/a-h/templ/safehtml	(cached)
ok  	github.com/a-h/templ/turbo	(cached)

But to be honest, i have no idea what are the possible implications of that change really are.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request NeedsInvestigation Issue needs some investigation before being fixed
Projects
None yet
Development

No branches or pull requests

4 participants