a-h / templ

A language for writing HTML user interfaces in Go.
https://templ.guide/
MIT License
8.04k stars 264 forks source link

implemented JsExpression type #851

Closed cornejong closed 1 month ago

cornejong commented 2 months ago

Description:

This PR implements the JsExpression type for using JavaScript expressions in script template arguments, as proposed in #850 with some modifications based on discussions and insights.

Objective:

The main objective, as described in #850, was to allow access to event objects in on* type event handlers applied on elements. This implementation enables passing any JavaScript expression to a script template.

Usage

script OnTheClick(event templ.JsExpression, element templ.JsExpression, randomExpressionResult templ.JsExpression) {
    console.log(event, element, randomExpressionResult)
}
templ TheButton() {
    <button onclick={ OnTheClick( templ.JsExpression("event"), templ.JsExpression("this"), templ.JsExpression("1 + 2") ) }>TheButton</button>
}
cornejong commented 2 months ago

Maybe, looking at it now. JsVar should be renamed to JsExpression. Since it basically would allow for any JavaScript expression to be passed this way.

That might be a better way of describing its behavior.

Thoughts and insights on this are very welcome.

garrettladley commented 2 months ago

Agreed @cornejong, JsExpression seems to be more reflective of the functionality. Curious to hear @a-h's thoughts

joerdav commented 2 months ago

Just for information in this PR, script templates is currently deemed as a legacy feature, not recommended as an approach anymore. https://templ.guide/syntax-and-usage/script-templates#script-templates

The reason was exactly that it wasn't flexible feasible to support all the different ways that javascript is used in modern web development.

The recommendation now is to use script tags, or js assets to provide scripting abilities. That way we can focus on the dx of editing js within html.

I hope this helps. But it may mean that this PR will not be accepted, @a-h would you agree?

cornejong commented 2 months ago

Thanks for the info @joerdav, Somehow i missed that (quite big) alert in the docs 😅 Although i realy like the script templates for small simple projects, if it's considered legacy i could see why it won't be accepted. I personally would still like to see it added, but we'll see how its goes.

For now i'll just leave the PR here and let @a-h decide it's fate.

iambpn commented 2 months ago

@joerdav yes it's true, that script method in templ is not recommended, but usage of script method is still the recommended way to handle on* HTML attribute like onClick, onChange and while working with event attribute access to this and event is a must.

As you mentioned, there is a workaround with <script> tag and templ.NewOnceHandle but with its own limitation of not being able to pass data from go to js directly.

for example this would not be possible if we use <script> tag:

templ ShowText(txt string) {
    <div>
        <input
            type="checkbox"
            onChange={ descriptionClicked(txt) }
        />
    </div>
}

script descriptionClicked(text string) {
    console.log(text)
}

Source: templ with Js Attribute

joerdav commented 2 months ago

I would disagree @iambpn I think that this is possible, but is a slightly different syntax.

If you are happy to take on a new dep you could use https://github.com/gnat/surreal

templ ShowText(txt string) {
    <div>
        <input
            type="checkbox"
            data-text={ txt }
        />
        <script>
            me("-").on("change", evt => console.log(me(evt).attribute('data-text')))
        </script>
    </div>
}

This can also be done with no deps

templ ShowText(txt string) {
    <div>
        <input
            type="checkbox"
            data-text={ txt }
        />
        <script>
            const comp = document.currentScript.closest('div')
            const checkbox = comp.querySelector('input')
            checkbox.addEventListener('change', function() {
                console.log(checkbox.dataset.text)
            })
        </script>
    </div>
}
iambpn commented 2 months ago

@joerdav this is also a great workaround, but it seems little trivial to go through dataset property. I would love to have this and event access in script method, but if this is the recommended way moving forward, I have no issues.

a-h commented 2 months ago

Looking good. I think this is a good solution to some of the problems. It doesn't deal with async functions or other issues, but it does unlock the use of passing event objects etc. to the functions, and it's not a big maintenance burden.

cornejong commented 2 months ago

@a-h Thanks!

I've committed your suggestions and chanaged the file names. In a bit i'll add it to the generator test suite and the examples to the docs.

cornejong commented 1 month ago

@a-h, tests and docs are added.

Out ot quriosity. You mentioned "It doesn't deal with async functions or other issues". What are currently some JS issues? I had a look in the github issues but i couldn't find any open issues.

a-h commented 1 month ago

Re: script template issues, at the moment, for example, with script templates there's no way to:

a-h commented 1 month ago

Thanks!

cornejong commented 1 month ago

Re: script template issues, at the moment, for example, with script templates there's no way to:

  • Put multiple functions in a script template
  • Write async functions
  • Use TypeScript or other build processes
  • Include NPM packages
  • Format JS code
  • Get LSP functionality for JS

Alright, yeah I've ran into some of these a few times. Have some idea's about some of them. l'll see if i can come up with some elegant solutions in my spare time.

a-h commented 1 month ago

If you want to catch up on a video call etc. to bounce ideas, can do that...

cornejong commented 1 month ago

If you want to catch up on a video call etc. to bounce ideas, can do that...

That might be nice, but I'll first tinker around a bit. See if any of these ideas hold water.

I've already played around with a really simple/bare bones typescript addition (simple compilation using tsc) using the "typescript" keyword in place of "script". But that leaves much to be desired.

a-h commented 1 month ago

Take a look at https://github.com/a-h/templ/issues/838 - I've just added some ideas at the end of that, riffing on what you've already added.