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

generator: Spread under attribute #611

Closed
iamajoe opened this issue Mar 12, 2024 · 7 comments · Fixed by #768
Closed

generator: Spread under attribute #611

iamajoe opened this issue Mar 12, 2024 · 7 comments · Fixed by #768
Labels
enhancement New feature or request generator NeedsInvestigation Issue needs some investigation before being fixed

Comments

@iamajoe
Copy link
Contributor

iamajoe commented Mar 12, 2024

There is a case where you might want to have a spread on an attribute.
Example:

<button class={ []string{"foo"}... }>Button</button>

This error comes in if i use this:

missing ',' in composite literal

If I generate like this instead:

<button class={ []string{"foo"}[0] }>Button</button>

Creates:

var templ_7745c5c3_Var2 = []any{[]string{"foo"}[0]}

Looking at the generation, it seems that supporting spread operator shouldn't be a problem. Some kind of parsing issue?!

@alehechka
Copy link
Contributor

alehechka commented Mar 15, 2024

I think for this case, using strings.Join from the standard library would likely be the simpler option to support the result you're looking for.

<button class={ strings.Join([]string{"foo"}, " ") }>Button</button>

@iamajoe
Copy link
Contributor Author

iamajoe commented Mar 15, 2024

For that specific case yes but the problem appeared when i was trying to use templ.CSSClass on TemplUI. It would be possible to create a join in there but probably better to support the go spread syntax since there might exist other similar cases. In my opinion of course.

@joerdav
Copy link
Collaborator

joerdav commented Mar 15, 2024

@iamajoe Would the CSSClasses function work?

<button class={ templ.CSSClasses([]any{"foo"}...) }>Button</button>

This should work for slices of not only strings.

@iamajoe
Copy link
Contributor Author

iamajoe commented Mar 15, 2024

@iamajoe Would the CSSClasses function work?

<button class={ templ.CSSClasses([]any{"foo"}...) }>Button</button>

This should work for slices of not only strings.

Hm. I haven't tried. I will take a look. Still not the best solution in my opinion since it is one more abstraction you need to know on top of the syntax but I understand the difficulty in implementing what I am talking about.

@joerdav
Copy link
Collaborator

joerdav commented Mar 15, 2024

On the contrary, I think this is one less abstraction to remember.

A spread inside curly braces is not something you would see in Go { []string{"foo"}... }. Spread really is only used to spread into a variadic function such as templ.CSSClasses([]any{"foo"}...).

Although I do understand this requires knowledge of a new function.

@iamajoe
Copy link
Contributor Author

iamajoe commented Mar 15, 2024

type Props struct {
  ClassNames []string
  CSS []templ.CSSClass
}
// ...
<div class={ props.ClassNames, templ.CSSClasses(props.CSS...) }></div>

And the error:

invalid use of ... in conversion to templ.CSSClasses

I could use templ.Classes but it cant do a conversion from []templ.CSSClass to []any for some reason.

For this to function I had to do this nastiness:

func convertCSSClassToClasses(css ...templ.CSSClass) templ.CSSClasses {
	var cssClasses []any = make([]any, len(css))
	for i, v := range css {
		cssClasses[i] = v.(any)
	}

	return templ.Classes(cssClasses...)
}

I guess templ.Classes could be a generic. i could work on that if you want.

@joerdav
Copy link
Collaborator

joerdav commented Mar 15, 2024

I wouldn't be opposed to classes being generic.

Another approach would be to just support []templ.CSSClass in the css processor https://github.com/a-h/templ/blob/main/runtime.go#L178

That way you could do:

<div class={ props.ClassNames, props.CSS }></div>

You wouldn't even have to spread.

@joerdav joerdav changed the title Spread under attribute generator: Spread under attribute Mar 25, 2024
@joerdav joerdav added enhancement New feature or request generator NeedsInvestigation Issue needs some investigation before being fixed labels Mar 25, 2024
@a-h a-h closed this as completed in #768 Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request generator NeedsInvestigation Issue needs some investigation before being fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants