Pre-RFC: Support one hbs comment format instead of two?

[GavinJoyce]

We currently support two .hbs comment syntaxes:

{{! this is the first supported syntax }}
{{!-- this is the second supported syntax --}}

Here's the AST output for both:

https://astexplorer.net/#/gist/e39b9bed0d6d7ce03d7c00a371cf016f/8ac50f526ebf1f68866ed31e387349d2cab4d559

Is there a reason to support both syntaxes? If not, perhaps we should open an RFC to deprecate one?

[rwjblue]

The {{! syntax does not support the comment itself including }} (close mustaches), but the {{!-- syntax does. It seems to me that it probably makes sense to always prefer {{!--.

We have a few options:

• create a lint rule to flag on {{! (without --) and add that to the recommended config (in the next major version of ember-template-lint)
• add a deprecation that is issued upon template compilation
• both, in a phased rollout system where the lint rule is added first then "sometime later" we add the actual deprecation.

I personally am in favor of the first and third (I don't think the deprecation on its own is very good ergonomics).

[pzuraq]

Would it make sense to deprecate handlebars comments altogether in favor of HTML comments? I realize Handlebars comments are removed, but it seems like HTML comments would be possible to remove as well in the same way, I wasn’t sure why we needed both options.

[rwjblue]

Hmm, I don't think so @pzuraq. There is actually (sometimes) value in emitting HTML comments (e.g. for instructions when inspecting the DOM). In addition, it would be a bit of an uphill battle from a SemVer perspective (since it would be a breaking change to stop emitting HTML comments).

[ghost]

I checked https://github.com/ember-learn/guides-source for usage patterns using rg '\{\{!' guides/release/. For both master and octane branches, {{!-- occurs most. {{! occurs only once in handlebars-basics.md

[pzuraq]

I'm not as convinced of the value of emitting HTML comments, you could make the same argument about JS comments, but we still minify and remove them for production builds. I think in most cases, they represent extra bytes without extra value.

Definitely agree that it's a much harder change from a SemVer perspective though. Probably something that would have to be done in time, so I agree this is probably better to tackle first either way.

[kellyselden]

@pzuraq When I teach others about handlebars, I explain "use html comments if you want them in the build, use handlebars comments if you want them stripped out". Just wanted to give you context of how some people understand them.

[pzuraq]

@kellyselden totally understand the difference, and how we teach it. I just haven't personally come across a use case where I actually wanted people to read HTML comments (where I also wouldn't want them to be able to read JavaScript comments).

I guess I feel like this is something that should be handled by a Handlebars/HTML minifier in production builds, rather than by a language level feature that requires us to teach people more things.

Edit: If anyone has use cases that demonstrate the value here, I'd love to surface them, would be good to know! 😄

[rwjblue]

@pzuraq - I totally agree that an HBS minifier should remove HTML comments, however this is a bit of a different thing (its completely opt-in by adding/using a minifier).

[chriskrycho]

HTML comments are also sometimes very useful semantically, because they let you wrap lines in an editor without adding a newline and therefore white space to the rendered output. As weird as that behavior is, it's also one I've needed to rely on several times.

[lougreenwood]

I always assumed that {{! }} was a shorter format for single line or inline comments. I often use this to take up less space:

{{! quick single line comment }}

<My Component
    @someArg={{this.thingy}} {{! some comment about arg}}
/>

{{!--
    Long
    Multiline
    Comment
--}}

[lougreenwood]

Not trying to side track - but if we're going to clean up comment behaviour, I'd LOVE to be able to wrap a comment in a comment - or rather comment out code which has a comment somewhere in it:

{{!--

    <p>Some stuff</p>
    {{!-- <p>Some stuff</p> --}}
    <p>Some stuff</p>

--}}

I often have to resort just deleting code which I want to comment out when debugging and that's just not safe at all... :(

[neojp]

@lougreenwood I haven't tested this, but I always assumed one was a single-line comment, and the other a multi-line comment; therefore you could nest single-line inside a multi-line comment.

{{!--

    <p>Some stuff</p>
    {{! <p>Some stuff</p> }}
    <p>Some stuff</p>

--}}

I always end up writing code like this while refactoring in other languages.

/*

    const foo = 'bar';
    // const bar = 'foo';
    const foobar = 'hello world';

*/

I am good for preferring one over the other, but I don't see the value of removing it if one is a single-line comment, and the other a multi-line comment.

[locks]

I think the third approach mentioned by @rwjblue is the best way forward.

[dcyriller]

Thank you @GavinJoyce for opening this discussion. I've wondered for some time why there were two syntaxes for multi-line comments, I was not alone ^^.

I'm okay to help / pair on opening a RFC for @rwjblue's third option. As it is a handlebars.js feature, I'm not sure where the deprecation would have to be issued though.

Not trying to side track - but if we're going to clean up comment behaviour, I'd LOVE to be able to wrap a comment in a comment - or rather comment out code which has a comment somewhere in it:

This feature is supported today thanks to the two syntaxes: {{!}} can be wrapped in {{!----}}. But if {{!}} is deprecated, nesting a comment in another won't be possible anymore. Would it be a blocker?

[wagenet]

I'm closing this due to inactivity. This doesn't mean that the idea presented here is invalid, but that, unfortunately, nobody has taken the effort to spearhead it and bring it to completion. Please feel free to advocate for it if you believe that this is still worth pursuing. Thanks!