casid / jte

Secure and speedy templates for Java and Kotlin.
https://jte.gg
Apache License 2.0
748 stars 56 forks source link

Possibly a bug(s) in the IntelliJ IDE plugin and JTE itself #257

Closed bohdan-shulha closed 11 months ago

bohdan-shulha commented 1 year ago

Hi @casid ,

We're doing a little bit strange thing: we wanted to save a little bit of efforts and we want to generate JSON with JTE.

Sorry for the screenshot, for now it is only to discuss what could possibly go wrong.

We have the following code and IDE tells that it can't parse the expression. However, the template itself works very well: telegram-cloud-photo-size-2-5359697089363300886-y

Among that, the indentation made by the built-in formatter is a little bit messed up, but that's not a big issue. :D

I believe that JTE (and IDE plugin) are not "looking" at the type annotation of the script tag and are treating all script tags as JS code.

I had similar issue when I was creating the importmap (it is json as well) and I had to $unsafe paths to the scripts like this:

    <script type="importmap">
    {
        "imports": {
            "alpine": "https://cdn.jsdelivr.net/npm/alpinejs@3.x.x/+esm",
            "sortablejs": "https://cdn.jsdelivr.net/npm/sortablejs@1.x.x/+esm",
            "@ess/critical": "$unsafe{staticUrl("/js/critical.js")}",
            "@ess/admin/common": "$unsafe{staticUrl("/js/admin/common.mjs")}",
            "@ess/admin/components/relation-picker": "$unsafe{staticUrl("/js/admin/components/relation-picker.mjs")}",
            "@ess/admin/components/tournaments": "$unsafe{staticUrl("/js/admin/components/tournaments.mjs")}"
        }
    }
    </script>

P.S. Do you have any kind of "BuyMeACoffee" link? You're doing a great job and I'd love to support you with a coffee-sized donate. Hope I'm not alone. ;)

kelunik commented 1 year ago

Please, for your own sake, use a proper JSON library for this task.

casid commented 1 year ago

Yes, jte treats all <script> blocks as JavaScript. And 100% want to second @kelunik here. Producing JSON with a template engine is a nightmare to maintain.

Still, a few thoughts.

Do you have any kind of "BuyMeACoffee" link?

I don't, I'm doing this for fun :-) But thank you very much for the heads up!

bohdan-shulha commented 1 year ago

Producing JSON with a template engine is a nightmare to maintain.

Yeah, I know. I needed a quick start for one of our devs, so I have allowed this, mostly as a temporary solution.

This should not produce an error: "position": "${loop.getIndex() + 1}"

That's true - it works, but, unfortunately, consumer side expects an integer. 🤷

Is there a reason you're using unsafe so much?

As for importmap - yes, unfortunately. If I omit the unsafe - JTE starts escaping slashes and instead of /some/file.js I'm getting \/some\/file.js.

Also, we've got a strange escaping with this case:

"name": "Top-G Makes History ..." without unsafe will be "name": "Top\-G Makes History ...".

All those escaping points make me think that this is just because JTE/Idea plugin are not expecting that <script> tag may contain JSON and apply the JS rules instead. 🤷

TBH, I didn't expect that these issues will be fixed - for sure, very special case, but I wanted to get some discussion on this and make the "problem" visible. :)

kelunik commented 1 year ago

mostly as a temporary solution.

There's no such thing, most temporary solutions will live forever. 😄

@casid As I said earlier, we shouldn't support variables in script tags at all and just make it a compile time error.

bohdan-shulha commented 1 year ago

We'll remove this hack along with the migrations of other pages from the heavy react ecosystem to jte. :)

I promise. :D

@casid As I said earlier, we shouldn't support variables in script tags at all and just make it a compile time error.

Why so strict? There are cases when you need to inject variables into JS, otherwise the solution will take way more efforts or would be less obvious.

Live example: we have a shim to "connect" old js-heavy frontend with the new one - extra lightweight built with jte&htmx.

<script>
    window.STATIC_URL = "$unsafe{staticUrl("")}";
</script>

We have the helper staticUrl which produces the link to the asset on CDN, depending on the build number. For example: staticUrl("/some.css") => https://super.cdn/f123rfa1/some.css.

If we'd not have the possibility to embed variables right in js, we'd need to create a helper function that will do lots of concatenations (mix-up of HTML. JS & Java), will be more error-prone, take more time to implement and support, and so on.

The same stands for ld-json & importmaps. Currently we can create a simple helper that would convert data structure in json and use it like this:

<script type="importmap">${json(Map.of( ... ))}</script>

<script type="ld-json">${json(List.of( ... ))}</script>

Yet again, without this feature it would force us to create needless abstractions.

At the very end, this is just the template engine and you'd never know the needs of the end-users. Can't get why to add artificial limitations. 🤷

casid commented 1 year ago

All those escaping points make me think that this is just because JTE/Idea plugin are not expecting that Githubissues.

  • Githubissues is a development platform for aggregating issues.