bigskysoftware / htmx

</> htmx - high power tools for HTML
https://htmx.org
Other
35.36k stars 1.18k forks source link

Additional braces added to JavaScript passed to eval #2619

Open Sjord opened 3 weeks ago

Sjord commented 3 weeks ago

https://github.com/bigskysoftware/htmx/blob/master/src/htmx.js#L2902-L2917

if (str.indexOf("javascript:") === 0) {
    str = str.substr(11);
    evaluateValue = true;
} else if (str.indexOf("js:") === 0) {
    str = str.substr(3);
    evaluateValue = true;
}
if (str.indexOf('{') !== 0) {
    str = "{" + str + "}";
}
var varsValues;
if (evaluateValue) {
    varsValues = maybeEval(elt,function () {return Function("return (" + str + ")")();}, {});
} else {
    varsValues = parseJSON(str);
}

The first part makes it possible to use js: or javascript: to supply values, instead of using JSON. If I understand correctly, this makes it possible to do something like this:

<div hx-get="/foo" hx-headers="js:createHeaders()">

However, curly braces ({, }) are added around the payload, even when it is JavaScript. So htmx tries to execute the following:

({createHeaders()})

and this is invalid syntax.

samfurr commented 3 weeks ago

Checking how the tests for this are written, this looks like expected behavior, though the docs don't make it super clear... The expectation is that you would provide something like "js:header1: getValue()" which as a whole would be evaluated and turned into the right format {"header1": "someReturnedValue"}; Again assuming here, but this may be because headers are inherently key/value based. If this isn't the expected behavior, I can make a PR update the tests and the method.

I do like the idea of passing in just a function call though, makes sense for passing collections of headers without extra JS cruft.

Sjord commented 3 weeks ago

this looks like expected behavior, though the docs don't make it super clear

Yes, it could very well be that I try to use this incorrectly. Describing how to use this in the documentation could be a solution for this.

I do like the idea of passing in just a function call though

It seems that this is currently possible with the spread operator:

js:...createHeaders()

So this all seems to be versatile and function fine, except that the syntax in both js:...createHeaders() and js:header1: getValue() is not intuitive and not documented.

samfurr commented 2 weeks ago

Created a PR to update the documentation and make JS evaluation work as expected where available through the underlying function: https://github.com/bigskysoftware/htmx/pull/2673