creationix / haml-js

Haml ported to server-side Javascript. This is a traditional server-side templating language. Tested with node-js
MIT License
902 stars 110 forks source link

probs with interpolation in attributes #41

Closed tribalvibes closed 13 years ago

tribalvibes commented 13 years ago

Thanks for putting this together--perfect timing for us :)

I'm trying to find the easiest way to conditionally include an attribute, and running into the following. E.g.,:

    - var target = thumbnail_url ? "target=\\"_blank\\"" : null
    %a( href="#{id}" #{target} )= name

This seems really awkward and so I am thinking of hacking the attribute parser to add some sort of conditional interpolation. Any thoughts on this? But, unfortunately, even the above does not work as expected and instead produces:

   var _$output="";var target = thumbnail_url ? "target=\"_blank\"" : null; _$output = _$output  +
"<a href=\"" +
html_escape(id) +
"\">" + 
name + 
"</a>";

which clearly (note the lack of interpolation of #{target} and the dangling quote ") seems like a bug, no?

Thanks! tv/

mikong commented 13 years ago

In regular HAML, attributes can be represented in brackets {}. If an attribute evaluates to false or nil, they are not included in the attributes. I think this is how it should be handled in haml-js. Null or false attributes must not be displayed during render. Right now, an attribute with null value will display the attribute with "null" string. I'm not sure if interpolation should be provided when proving attributes in parenthesis.

mikong commented 13 years ago

Correction on my last comment, it looks like null, false, and undefined does work. But if you input an expression like

%option{value: '1', selected: (a == b ? 'selected' : false)}

Even though the expression evaluates to false, selected attribute is rendered with "false" string instead.

aaronblohowiak commented 13 years ago

To fix this is very hard.

Here is why: Haml-JS renders your template string to a function that is self-contained. That is, the result of compiling a template string with Haml-JS returns a function that is just string concatenation and the contents of what you pass in as the locals. When you have a definition like this:

%option{value: '1', selected: (a == b ? 'selected' : false)}

It is hard to know if the value of the "selected" attribute, "(a == b ? 'selected' : false)" is something that should be determined at run-time or at compile time. There is a special-case check for false, null, undefined or "" at compile time that will cause the attribute to be elided.

If you want to try and fix this issue, I would accept a patch to render_attribs.

I am going to mark this issue as Closed.

costa commented 11 years ago

This is a real issue with pretty basic functionality. I don't think this ticket should have been closed.

aaronblohowiak commented 11 years ago

If you want to try and fix this issue, I would accept a patch to render_attribs.

costa commented 11 years ago

@aaronblohowiak Man, I've read your comment right above mine and I still don't think it should be closed — people who use this project must be aware of its limitations, don't you think? I've just burnt some time over this issue which could be avoided if I knew there's a problem with this.

To submit a patch or to not submit — that's a completely different question. Modules of this size have usually infeasible study-fix ratio, so patches are submitted by people that want something really bad. Here, it is just a matter of postponing evaluation of an expression defining a property — it shouldn't be too hard for someone familiar with the code.

aaronblohowiak commented 11 years ago

Would also accept a documentation patch.

I know you think it "shouldn't be too hard", but there are a couple things you should be aware of about haml-js: it was a prototype, it needs to be re-written, it is not based on proper compiling principles. @creationix did a great thing putting this together, and I have tried to extend it and clean up where it made sense and have added a few more test cases -- but he and I both have primarily moved on to other things.

On Tue, Feb 26, 2013 at 9:40 AM, Costa Shapiro notifications@github.comwrote:

@aaronblohowiak https://github.com/aaronblohowiak Man, I've read your comment right above mine and I still don't think it should be closed — people who use this project must be aware of its limitations, don't you think? I've just burnt some time over this issue which could be avoided if I knew there's a problem with this.

To submit a patch or to not submit — that's a completely different question. Modules of this size have usually infeasible study-fix ratio, so patches are submitted by people that want something really bad. Here, it is just a matter of postponing evaluation of an expression defining a property — it shouldn't be too hard for someone familiar with the code.

— Reply to this email directly or view it on GitHubhttps://github.com/creationix/haml-js/issues/41#issuecomment-14128316 .

skatenerd commented 9 years ago

FWIW, I stumbled across this issue and reading it saved me a lot of time.