glimmerjs / glimmer-vm

MIT License
1.13k stars 191 forks source link

Incorrectly removing third curly bracket #1405

Open AW0005 opened 1 year ago

AW0005 commented 1 year ago

Experiencing this in Prettier's usage of Glimmer as reported here: https://github.com/prettier/prettier/issues/14090#issuecomment-1370760731 Where they state it is an issue in Glimmer itself.

Trying to add triple curly braces to be able to escape html content, but the third brace is removed if it's inside an html element.

onlyIf is a custom helper I am using on my project that evaluates on argument one, and then conditionally outputs argument two for context.

Input:

<input
  name="is-active"
  type="checkbox"
  value="active"
  {{{onlyIf active 'checked="checked"'}}}
/>

Output:

<input
  name="is-active"
  type="checkbox"
  value="active"
  {{onlyIf active 'checked="checked"'}}
/>

Expected behavior: Output:

<input
  name="is-active"
  type="checkbox"
  value="active"
  {{{onlyIf active 'checked="checked"'}}}
/>
Windvis commented 1 year ago

Does the code work before Prettier strips the extra braces?

Curlies in that position are considered modifiers not "Mustache statements" so I don't think it can do what you are trying to do.

Would something like this work for you? It should have the same result.

<input
  name="is-active"
  type="checkbox"
  value="active"
  checked={{active}}
/>
AW0005 commented 1 year ago

@Windvis Yes the code works before prettier strips the extra braces (and I am currently getting around this issue by using Handlebars.SafeString but I shouldn't have to do that.

Unfortunately the code you shared does not give me the same result. Instead it makes checked equal to either "true" or "false" and 'checked'="false" still causes an html checkbox to be checked, you need to not have the attribute present at all for it to be unchecked.

To be extra clear as well I am coding in vanilla handlebars, and not using glimmer directly for parsing. I am only using it as a byproduct of it being how prettier formats handlebars files.

Windvis commented 1 year ago

@AW0005 Ah, I see. I assumed you where trying this in an Ember / Glimmer VM project (in which case the code snippet would trigger a build error and the snippet I posted would work as expected).

The Handlebars implementation in Ember / Glimmer is not the same as Handlebars.js. Prettier only includes support for the Glimmer subset so you would need a Handelbars.js specific Prettier plugin, but I'm not sure if that exists already.

AW0005 commented 1 year ago

@Windvis Yea unfortunately it does not - prettier themselves lists glimmer as their official handlebars support. I'll go back to them with this and see if they might not be able to add some kind of option on top for this somehow.