Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion for commenting out HTML attributes #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

akauppi
Copy link

@akauppi akauppi commented Dec 12, 2020

We've briefly discussed this at Svelte Discord and since it seemed a welcome suggestion, I formulated the RFC.


Rendered

Edit: Added link to rendered document (pngwn)

@akauppi
Copy link
Author

akauppi commented Dec 12, 2020

Found HTML Comments inside Opening Tag of the Element
(SO). It essentially says no, one cannot have comments within an HTML tag, and suggests some work-arounds.

@pngwn
Copy link
Member

pngwn commented Dec 12, 2020

Parsing this syntax would be straightforward and it wouldn't be a breaking change.

Syntax highlighters don't typically parse so the change would need to made there as well.

The larger issues is the design, this would mean that we would have two wildly different syntaxes for comments inside and outside of the tag body. This is not particularly intuitive if your mental model for svelte is HTML (which is the one we push in official documentation), it only makes sense if your mental model for svelte markup is JSX.

@Conduitry
Copy link
Member

A similar suggestion came up a year ago on Discord as well, and I'm not thrilled about the new suggested syntax either.

@pngwn
Copy link
Member

pngwn commented Dec 12, 2020

If there is a desire to ignore attributes specifically an 'attribute comment prefix' could be considered. A flag that informs the Svelte compiler and any related tooling to ignore/ comment the attribute: _attr={boo}, ~attr={boo}, etc. This would probably be a breaking change.

I'm not totally convinced that the feature is necessary but this could be an alternative.

@pngwn
Copy link
Member

pngwn commented Dec 12, 2020

The above would also have the benefit of not being sensitive to lines allowing you to ignore attributes on the same line as others:

<Thing attr1={boo} ~attr2={boo} />

Which may be important, with things like prettier auto-formatting etc. If the desire here is to comment out props, attrs, and directives then any solution should specifically target those things. Line-based comments seem like a broad brush.

@pebezo
Copy link

pebezo commented Dec 12, 2020

This is not particularly intuitive if your mental model for svelte is HTML

You have {#if ..} ... {/if} blocks that are not HTML already... What I've seen in other server-side rendering frameworks (somewhat applies to Svelte) is to have a notion of server-side commenting. The {#if ..} bit doesn't make it to the browser, so we already have a "pre-processing" layer.

How about this: {#} to open a comment block and {/} to close it.

<div class="foo" {#} style="color:red" {/}> ... {#} server comment {/} ... <!-- client comment --> </div>
<Sample a=10 b=12
  {#} further=13 <-- disabled for testing {/}
/>

The actual syntax could be different of course. One could also have a VS Code extension that would help with the comment/uncomment parts.

@rchrdnsh
Copy link

rchrdnsh commented Dec 12, 2020

when I want to temporarily comment out an attribute at the moment I copy and paste it into the script section then comment it out there and then delete it in the html template, then I have to undo all that to put it back in...any of these proposed solutions would be fine with me, so long as I can use the keyboard shortcut to do so :-)

Although my intuition wanted to see <!-- foo='bar' --> the first time I tried to do this in a component...

@Monkatraz
Copy link

Syntax highlighters don't typically parse so the change would need to made there as well.

I made the new syntax highlighter for the VSCode extension, and I will say pretty much any of these suggestions would be fairly straightforward to implement. The most complex example would be the ~attr=foo form, but regardless isn't too difficult. I would actually recommend some other syntax, say /* comment */ instead, because that can just be used anywhere and won't be specific for attributes.

If I introduce even more of my opinions, I would think that just parsing JS syntax for comments in markup would be the most intuitive thing to do. You could then add support for JSDoc or whatever if you'd like.

@pngwn
Copy link
Member

pngwn commented Dec 12, 2020

I would actually recommend some other syntax, say /* comment */ instead, because that can just be used anywhere and won't be specific for attributes.

Unless there is a usecase outside of attributes I see no reason to find a catch all solution, we already have a syntax for commenting outside of tag bodies, inside the only thing that can exist are some kind of attribute/ property. We design for the problem we have, not for theoretical flexibility.

I would actually recommend some other syntax, say /* comment */ instead, because that can just be used anywhere and won't be specific for attributes.

I would think that just parsing JS syntax for comments in markup would be the most intuitive thing to do. You could then add support for JSDoc or whatever if you'd like.

This would introduce even more complexity, combining multiple syntaxes can seem attractive but you usually end up running into all kinds of edge-cases and limiting the actual text a user can use. HTML already has a series of characters that you cannot use without escaping them, supporting other JS or JS-related syntax would make that even worse.

This is actually the main reason I'm against this proposal. We currently support two things in the markup: HTML and Svelte-specific syntax. Adding a third, especially one with such a large set of related tools and syntaxes, could set unrealistic expectations and add more confusion.

@Monkatraz
Copy link

Monkatraz commented Dec 12, 2020

My viewpoint on it goes like this: If Svelte were to deviate from HTML, what would be most intuitive? In my opinion, that's simply duplicating the comment syntax already used by both CSS and JS, /* comment */. However, in principle, I entirely agree with the idea that by doing this, the HTML+Svelte syntax becomes more complex, perhaps needlessly.

...but if you're going to do it, I think you should just duplicate the syntax you're already using. Otherwise, I'm neutral on this and implementing it for IDEs is pretty simple, mostly a copy paste job.

We design for the problem we have, not for theoretical flexibility.

In regards to this, I think introducing new syntax that must be documented somewhere and that's only useful for one thing (commenting attributes) is a waste of the effort needed, basically. But that was kind of your point, wasn't it?

@Monkatraz
Copy link

Monkatraz commented Dec 12, 2020

Oh also, just to throw more stuff at the wall, if Svelte adds a symbol for ignoring an attribute, I think it would make the most sense to duplicate HTML comments instead:

<Thing attr1={boo} !--attr2={boo} />

I'm not sure if this would conflict with anything - I don't think it would just by casually thinking about it.

EDIT: Well okay, it would look odd for a styled component:

<Thing attr1={boo} !----my-color='red' />

But I don't see any reason why it wouldn't work anyways.

@pngwn
Copy link
Member

pngwn commented Dec 12, 2020

I think introducing new syntax that must be documented somewhere and that's only useful for one thing (commenting attributes) is a waste of the effort needed, basically.

That is the only use-case though. Everything else is already covered. Even if JS comments worked everywhere the only new thing would be commenting attributes. We would still support HTML comments because Svelte aims to be a superset of JavaScript, so we would also have two ways to do the exact same thing, which goes against one of our design principles in a major way.

Based on the above quote/ comment, the logical conclusion is that this feature wouldn't bring enough value and should not be implemented. The syntax would still need to documented and implemented even if we went with JS/CSS comments because there is no reason to expect it would work from a user's point of view; Svelte markup does not support JavaScript syntax at all, this would be the exception. Custom syntax has the benefit of not having any existing associations causing confusion (I'm not actually proposing this, just an alternative).

@Monkatraz
Copy link

Monkatraz commented Dec 12, 2020

I'm pretty sure we're effectively in agreement, but just for the sake of discussion:

That is the only use-case though. Everything else is already covered.

Personally, I am a fanatic for terse syntax and dislike the fairly bulky <!-- comment --> syntax, and introducing // comment would be a feature I'd use constantly.

The syntax would still need to documented and implemented even if we went with JS/CSS comments because there is no reason to expect it would work from a user's point of view; Svelte markup does not support JavaScript syntax at all, this would be the exception. Custom syntax has the benefit of not having any existing associations causing confusion.

I think once a user finds out that the syntax is supported, it immediately becomes obvious. I'm not really sure what a user might get confused about, honestly.

A bigger issue is that this (full JS comments) would be a breaking change, for example it could alter unescaped text found inside of <code> tags. (which I realize you mentioned in the form of edge-cases, but this is a direct edge-case that would come up frequently)

@JBusillo
Copy link

If I want to 'comment out' an attribute, I usually just change the attribute name. E.g., if the original code is:
<button on:click={x => console.log("click")} attr1={name}>
... I'll prepend an eye-catcher (in this example, "xxx") such as:
<button on:xxxclick={x => console.log("click")} attr1={name}>
... or
<button on:click={x => console.log("click")} xxxattr1={name}>

Occasionally I make the mistake of using the VSCODE "Toggle Line Comment" (CTRL + '/') inside an HTML element. But this inconvenience isn't unique to .svelte files -- it applies to any file containing HTML.

Unless I'm missing something, I don't see the need to stray further from the HTML specification when there is an easy workaround.

@pushkine
Copy link

pushkine commented Dec 13, 2020

I thought about this and I believe that the better solution would be to do it through the svelte extension

I guess we all use the vscode shortcut that comments out lines of selected text, I suppose that shortcut executes a command that could be handled by the svelte language server

@akauppi
Copy link
Author

akauppi commented Dec 13, 2020

It was nice to see the wealth of discussion on this - that's the reason I created the RFC in the first place.

I think I'll go to simply using __DISABLED="..." like convention in my own code (requires no changes to Svelte).

One thought about the syntax options arose on @pngwn 's comment:

This is actually the main reason I'm against this proposal. We currently support two things in the markup: HTML and Svelte-specific syntax. Adding a third, especially one with such a large set of related tools and syntaxes, could set unrealistic expectations and add more confusion.

That's so true. If we stick to the two existing ones, and if there is a wish to be able to comment out attributes, in situ, there is this syntax option:

 <Draggable
   {x} {y}
   bind:m={ matrices[i] }
   {//on:some      this whole line would be ignored}
   {/* maybe multiline,
   too? */}
 >

The expressions within { ... } are JavaScript, but they don't allow comments (or lack of the expression). This can be changed.

  • {// ...} would be seen as {}
  • {/* ... */} would also be seen as {}

I presume it is intentional that comments are currently not supported within the expressions. HTML attributes are intended to be short and self-explanatory. But in the context of Svelte (where there's no penalty for inline handlers) that's a bit artificial. One could make multiple-line handlers (it seems to pass the build) and if so, there may be use for having comments among that source.

If there is interest in this direction, I am happy to edit the original RFC to reflect this syntactic approach. If the overall consensus is that the feature is not needed, that's it.

@CherryDT
Copy link

CherryDT commented Mar 14, 2021

The workaround I use for this at the moment (and which may be useful for others looking at this because they were trying to find a way to write a comment in the current version already) is {...{/* comment */}} which works because {...{}} is valid and just passes zero props.

I use this not only to "comment out" things like the original use case was described in this RFC but also for adding comments about specific attributes, especially in case of multiline opening tags with several attributes each occupying a line.

Being ignorant of this existing RFC I opened sveltejs/svelte#6087 before, independently suggesting the same {/* comment */} syntax as you did, akauppi, so count this as a +1. (And if sveltejs/svelte#6086 were to parse as empty, maybe emitting a warning if it is totally empty even without any comment, we would be there, I guess.)

@arxpoetica
Copy link
Member

Throwing my hat in the ring...

As someone who has wanted something like this in the past. Having read through all the comments and thinking about how it detracts from the "stay as close to HTML/JS/CSS" philosophy of Svelte...

I'm fairly opposed to this.

I would be more inclined to have something similar end up in tooling, as @pushkine proffered.

@brandonmcconnell
Copy link

brandonmcconnell commented Mar 24, 2022

That's so true. If we stick to the two existing ones, and if there is a wish to be able to comment out attributes, in situ, there is this syntax option:

 <Draggable
   {x} {y}
   bind:m={ matrices[i] }
   {//on:some      this whole line would be ignored}
   {/* maybe multiline,
   too? */}
 >

The expressions within { ... } are JavaScript, but they don't allow comments (or lack of the expression). This can be changed.

  • {// ...} would be seen as {}
  • {/* ... */} would also be seen as {}

I presume it is intentional that comments are currently not supported within the expressions. HTML attributes are intended to be short and self-explanatory. But in the context of Svelte (where there's no penalty for inline handlers) that's a bit artificial. One could make multiple-line handlers (it seems to pass the build) and if so, there may be use for having comments among that source.

If there is interest in this direction, I am happy to edit the original RFC to reflect this syntactic approach. If the overall consensus is that the feature is not needed, that's it.

As @CherryDT did, I also unknowingly opened an issue, sveltejs/svelte#7389, before realizing this RFC issue was already created.

I'll go ahead and close that one as a duplicate and continue my efforts in this ticket.

I would also like to re-propose the Svelte-syntax-like comment structure which would not only work well with the existing Svelte syntax, but it would also work identically to the industry standard for how JSX works, so much so that simply switching the syntax highlighting mode even here on GitHub from svelte to jsx causes the comments, whether inline or multi-line to start displaying as expected when wrapped in {/* and */}.

And finally, to add some notes from the issue I incidentally opened—

This is how that would look in practice:

Single-line tag with single-prop comment:

<SpaceForce {/*prop={"Boots on the moon."}*/} data={"Tuna sandwich"} />

Single-line tag with multi-prop comment:

<SpaceForce {/*prop={"Boots on the moon."} data={"Tuna sandwich"}*/} />

Multi-line tag with single-prop comment:

<SpaceForce
  {/*prop={"Boots on the moon."}*/}
  data={"Tuna sandwich"}
/>

Multi-line tag with multi-prop comment:

<SpaceForce
  {/*prop={"Boots on the moon."}
  data={"Tuna sandwich"}*/}
/>

And by the nature of Svelte’s syntax, using this same commenting syntax should also be supported for commenting entire components as well, since interpolating values with curly braces works outside of parent components and elements too:

Commenting entire element/component:

{/*<SpaceForce
  prop={"Boots on the moon."}
  data={"Tuna sandwich"}
/>*/}

All of these examples work when used in JSX and yet also follow a syntax structure nearly identical to what is already supported in Svelte. It would be great if Svelte supported the same. I do not see this as any major deviation from how Svelte currently works, and it is no more magic than many of the directives and syntax already introduced. If anything, this is less magic and more rudimentary.

@anatolzak
Copy link

Is there any progress?

@nabe1653
Copy link

nabe1653 commented Feb 8, 2024

I understand svelte template should not break html format but sometimes JSX/Svelte attr has more complex variables than pure html.

That JS comment style in bracket is very nice suggests, it will be smarter than commenting attr with some magic-value prefix.
I wish svelte adopts it as soon as possible.

@tivac
Copy link

tivac commented Feb 18, 2024

This is causing weird issues on m-css.com

image

@AlbertMarashi
Copy link

We could also support an option to treat underscore props as comments

<foo
  _class:foo={true}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.