ruricolist / spinneret

Common Lisp HTML5 generator
MIT License
369 stars 26 forks source link

with-html in spinneret/ps creates function call on class attribute(s) when one of them is a variable #47

Closed l0rdn1bbl3r closed 3 years ago

l0rdn1bbl3r commented 3 years ago

In spinneret (non ps) this works as expected.

(let ((classvar "myclass"))
    (spinneret:with-html
      (:div#myid :class classvar
                 (:p "lorem ipsum"))))

The html output is:

<div class=myclass id=myid>
 <p>lorem ipsum
</div>

But when I do this in parenscript

(ps:ps
  (let ((classvar "myclass"))
    (spinneret:with-html
      (:div#myid :class classvar
                 (:p "lorem ipsum")))))

the javascript output is:

(function () {
    var firstValue141;
    var valuesList142;
    var classvar = 'myclass';
    var node137 = window.spinneret || (window.spinneret = document.createDocumentFragment());
    var d138 = document;
    node137 = node137.appendChild(d138.createElement('div'));
    node137['className'] = [[classvar()].join('')].join('');
    node137.setAttribute('id', 'myid');
    node137 = node137.appendChild(d138.createElement('p'));
    node137.appendChild(d138.createTextNode('lorem ipsum'));
    node137 = node137.parentNode;
    node137 = node137.parentNode;
    if (!node137.parentNode) {
        var val140 = ((__PS_MV_REG = [], firstValue141 = node137), (valuesList142 = __PS_MV_REG.slice(), (valuesList142.unshift(firstValue141), valuesList142)));
        window.spinneret = null;
        var valuesList143 = val140.slice();
        firstval144 = valuesList143.shift();
        __PS_MV_REG = valuesList143;
        return firstval144;
    };
})();

This results in an error later since classvar clearly isn't a function.

Multiple class attributes don't change this. The important thing is that at least one isn't a literal string.

After having a look at ps.lisp in spinneret I think the issue is quite simple.

From ps.lisp.

(defpsmacro join-tokens (&rest classes)
  `(stringify
    ,(ps::concat-constant-strings
      (intersperse " "
                   (remove-duplicates (remove nil classes)
                                      :test #'equal)))))

ps::concat-constant-strings returns a list but ps::stringify only has a &rest arg and therefore when given '(foo) returns "(foo)".

I think that splicing the result of calling ps::concat-constant-strings is all there is to do. Like this.

(defpsmacro join-tokens (&rest classes)
  `(stringify
    ,@(ps::concat-constant-strings
      ...)))

I tried it and it seems to be working but I might have missed something of course.

ruricolist commented 3 years ago

Thanks for catching this. If you would like to make a pull request I would be happy to merge it.