Extend JSXText with Comment?

[syranide]

It may be worth extending JSXText with a case for Comment. In a way it's a bit weird as it is technically as if the comment in 'abc/*def*/ghi' would actually work. But practically, I think it makes sense, commenting out JSXElements is currently quite unintuitive/hacky as you have to write:

<compa>
  {/*This text should not be printed: */} value
  <compb>
    {//<compc></compc>} -- actually broken right now IIRC
  </compb>
</compa>

Note that comments actually affect (split) the children too, rather than being transparent. I'm proposing extending the syntax to support the following:

<compa>
  /*This text should not be printed: */ value
  <compb>
    //<compc></compc>
  </compb>
</compa>

As I mentioned, it is a bit weird in a way but still rather intuitive and useful, especially in the sense of having JSX feel like a true extension of JS and not just sugar. I think there is little to lose here; /*, */ and // are unlikely to occur in text, if they do then HTML-entities or JS-escaping (#4) to the rescue. Or if you don't care about children {'/*'} (half-related #6, leading/tailing whitespace).

[sebmarkbage]

If we do this then we should treat all text and attributes as JS encoded (i.e. allow backslash escapes) and not have special support HTML-entities.

Another alternative is to support XML style comments instead <!-- -->.

I think we need to make up our mind. Are we JS or are we XML? @jeffmo

[syranide]

@sebmarkbage Yep, I explored that idea a bit in #4. To me it makes perfect sense to drop HTML-entities and not pretend that JSX is HTML, JSX is great for any UI frontend, HTML just happens to be what the web is built on and what React currently targets. As I mention in that issue, it would also put an end to the weird <a href="&amp;\" /> vs <a href={'&\\'} />.

My personal opinion on <!-- --> is that it is neat from a technical perspective; it's not at all invasive. However, I think it only makes sense in the historical context of HTML, had HTML never been I think JS-style comments would've been the obvious go-to.

In my opinion JSX is currently 50% JS and 50% XML, being 100% JS would certainly be preferable. But it's your decision obviously :)

var result = <div value="a">text{<div />}<div />text</div>;
/*var result = <div value="a">text{<div />}<div />text</div>;*/
var result = /*<div value="a">text{<div />}<div />text</div>;*/
var result = <div /*value="a"*/>text{<div />}<div />text</div>;
var result = <div value="a">text{/*<div />*/}<div />text</div>;

var result = <div value="a"><!-- text{<div />}<div />text --></div>;
var result = <div value="a">text{<div />}<!--<div />-->text</div>;

... is kind of weird.

[ninjasort]

I agree about <!-- --> comments as they would take the place of a proper comment node. Thoughts on using a configurable jsx parsing option for allowing or disallowing // or /**/. One could set this boolean to true while developing and always turn it off in production allowing // and /**/ to come in as text at that point. facebook/react#3008

[jeffmo]

Are we JS or are we XML?

I am human. I am dancer.

[jeffmo]

There's no technical reason we couldn't support . The only reason we haven't is because we support comments in {} expressions (which was technically simpler and covered more flexibility).

My personal opinion: We should stick with the status quo. If we go with first class comments like this, we make it easy to erase text unintentionally (by accidentally having a '/*' or '//' in your text somewhere. Doesn't seem worth that to me...but I'll leave it up to the popular opinion to decide if everyone else agrees.

[jeffmo]

I also don't think is really that much better than {/* */}

(It's the same number of chars even)

[jeffmo]

Oh...this issue is super old...

[syranide]

@jeffmo I still think it's relevant though. But my point was that why even require {/* */}, why not make JSX behave like JS which is the host language. I.e. let // and /* */ work consistently through-out JSX as well, so that commenting works intuitively across all your source code. It's not like you'd expect to find // and /* in text and if you really need to output them then {'//'} to the rescue, just like it is for {, < and so on.

PS. I also think that <!-- --> should map to the comment node, not to source comments.

[jeffmo]

@camronjroe: Having a parser flag to turn this on/off would be a not so great idea. It would mean react component interop (meaning like sharing react components) wouldn't just work -- you'd have to worry about the parser settings each component relies on too. It would also mean weird little edge case bugs (like text not showing up, or showin up when you don't want) could pop up in application code that ultimately traces allll the way down out of your app code and into your build/compilation system. Turtles!

[jeffmo]

@syranide: I won't die on a hill against this, and I agree it's /maybe/ not common to have // or /*, but it's the times where it does that would make this really frustrating (it's easy to miss, and because it doesn't happen often...really hard to track down).

Anyway,

[syranide]

@jeffmo But that's partially what syntax highlighting is there for right? :)

[ninjasort]

@jeffmo In that case a flag may not be the way to go.

Considering the use of {} for embedding javascript it makes sense that {// } would be an embedded javascript comment. But one can't really use the // style comments because in your editor you will actually be commenting in two js contexts. One inside the {// } and one inside the js file itself. This raises an issue because it comments the entire line, hence forcing you to use {/* */} which has a closing comment.

The problems here are that js can't be fully embedded properly and it takes about 2+ seconds to do {/* */}. However, since it is emulating a XML like syntax, what would be wrong with using <!-- --> style comments? It's the correct type of comment in that context and there would be no need for flaky embedded js comments as it could comment the whole jsx node.

[syranide]

What I find mostly annoying about the current state of comments is that, this makes sense and works:

return (
  //<div />
);
return (
  <div
    //title="test"
    /*foobar=""*/
  />
);

But this doesn't:

return (
  <div>
    //<div />
    /*Text*/
  </div>
);

Instead you have to do:

return (
  <div>
    {//<div />}
    {/*Text*/}
  </div>
);

While they're largely the same, there's a mental burden in having to check the context and {/* is just one character longer, but it's unfamiliar and quite a lot more annoying to type as well (at least on swedish keyboards). {// is the same but also requires a closing } which again makes it even more annoying when wanting to make a quick comment.

Then there's also the problem that the current expression comments acts as a separator, so you cannot comment out in the middle of text without affecting the resulting children. It's not really all that common and a big deal, but perhaps yet another reason for why it's a bit weird.

<div>foo{/*tao*/}bar</div>

Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed.