jsx
jsx copied to clipboard
facebook
Extend JSXText with Comment?
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).
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
@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="&\" /> 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.
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. https://github.com/facebook/react/issues/3008
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.
I also don't think is really that much better than {/* */}
(It's the same number of chars even)
@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.
@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!
@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,
@jeffmo But that's partially what syntax highlighting is there for right? :)
@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.
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.
I am for XML comments as they make it much easier to comment something out in the JSX flow and keep it friendly to the human eye at the same time. JS comments inside markup just look strange. In an ideal JSX block you would have no or minimal JS control flow anyway. It would all be encapsulated in smaller components, wouldn't it?
<Foo>
<!-- I will tell you why this component is here -->
<Bar>
</Bar>
</Foo>
This would also allow us to quickly temporarily comment out components in the JSX flow like this:
<Foo>
<!--Bar>
</Bar-->
</Foo>
@jeffmo said "I also don't think is really that much better than {/* */} (It's the same number of chars even)". Technically it is one more character, but it doesn't have to be. For example, If I want to comment out this block of JSX:
<div>
<div>some text</div>
</div>
I would do it by adding just five characters like this:
<!--div>
<div>some text</div>
</div-->
But the number of characters isn't the point. I think their are four points.
- {/* ... */} is a new comment syntax to everyone. Why require a new syntax?
- That syntax requires typing 4 unique characters, but my example only required 2.
- JSX looks like XML, so it should support XML comments because people will naturally expect it.
- This is often raised as a reason people don't like JSX. The more of those reasons we can eliminate, the easier it will be to convince people to use React.
Please reconsider this. I'd be willing to work on a pull request to add support, but only if there's a good chance of it being accepted.
@mvolkmann IMHO, JSX is not XML, it's JS. It's just new syntax to do something we've always been able to do.
<!--div>
<div>some text</div>
</div-->
Is also problematic, because that would mean that JS(X) now has two ways to comment stuff.
var a = 1 <!-- x -->;
var b = 1 /* x */;
@mvolkmann IMHO, JSX is not XML, it's JS. It's just new syntax to do something we've always been able to do.
👍
...because that would mean that JS(X) now has two ways to comment stuff.
@syranide Given that JSX is a "new syntax to do something we've always been able to do", wouldn't this argument (JSX has two ways...) be an argument against JSX entirely? There's already "two ways" (at least) to do almost everything in JS(X), the plain JS way or with the new syntax. Why should comments be different?
This recommendation (as I understand it) is simply to make the second (X) way more consistent with XML, which is the expectation of almost any coder learning JS(X) in my opinion. It lowers the barrier to entry and makes for fewer awkward well, no, that actually doesn't work like you would expect moments when introducing folks to React.
Given that JSX is a "new syntax to do something we've always been able to do", wouldn't this argument (JSX has two ways...) be an argument against JSX entirely? There's already "two ways" (at least) to do almost everything in JS(X), the plain JS way or with the new syntax. Why should comments be different?
True, but the syntax we had for it is in many ways bad, it was hard to read especially for large hierarchies and cumbersome to write. It's also very alien to typical designers. There are also some technical differences.
Yes, there's some overlap there with <!-- --> for comments... but committing to introducing JSX elements (and thus a separate language) were not a decision people took lightly. It's hugely problematic in many ways but most agree that it was/is worth it. All it introduces is a new isolated syntax for creating "elements", the syntax is entirely voluntary and does not affect the language in any other way. Introducing a new comment syntax for "familiarity" increases the scope further and at a technical level makes absolutely no sense at all IMHO.
This recommendation (as I understand it) is simply to make the second (X) way more consistent with XML, which is the expectation of almost any coder learning JS(X) in my opinion. It lowers the barrier to entry and makes for fewer awkward well, no, that actually doesn't work like you would expect moments when introducing folks to React.
JSX elements is already different enough from HTML that learning to use JS-comments for commenting code should seem trivial, especially since you need to have some level of experience in JS anyway. The benefit of having a single way to comment is also important IMHO, see https://github.com/facebook/jsx/issues/7#issuecomment-53928414.
Also worth noting, x <!--a may actually be valid syntax in some languages (x < (!(--a))), but isn't in JS for some reason, not entirely sure why TBH...
I think it's one of those things that makes intuitive sense, but when you get down to it and actually start to evaluate the side-effects it no longer seems like good idea. That being said, if <!-- x --> actually returned a JSX comment element then that might make sense (the possible syntax ambiguity above needs to be considered though). Which may actually be what people expect since comments have meaning in some cases.
var a = 1 <!-- x -->;
var b = 1 /* x */;
That its not where the XML comment would be allowed to work. It would only be valid inside jsx markup.
Everybody coming from HTML will naturally try XML comments like they did in HTML. And they ask will fail when trying this in jsx. I did. You can explain them all day long that jsx is js and not html although it looks like html from an birds eye view. And while XML comments would be natural, they aren't JS.
Would be interesting to run a questionnaire with beginners about their first try to comment some jsx markup.
That its not where the XML comment would be allowed to work. It would only be valid inside jsx markup.
@pke But then you have to use different comment syntax depending on where are you in the code... having contextual comment syntax seems like over-complicating a fundamental language feature when the syntax we already have would work just as well. It also hurts refactoring.
return (
// outer component
<div>
<!-- inner component -->
<div />
</div>
);
Oftentimes you actually need comments emitted in HTML.
Such as adding notices into the source, debugging crazy off-by-one errors, inserting markers in the output for post-processing.
Forcing this kludge is against all that is human:
<div style={{display:none}} dangerouslySetInnerHTML={{__html: `
<!-- here is my comment
it was a hard fight wasn't it?
---> `}}></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.
Contextual comments are not just a mental burden, they completely break all pre-existing comment shortcuts in code editors.
I normally never actually type //, I just highlight several lines of code and hit ⌘+/ and every line is automatically commented out (or automatically uncommented). Likewise I never type /**<ENT><SPACE>*<ENT><SPACE>*/, I just type /**<TAB> and start writing my doc comment. That completely breaks in JSX. If any of my comments start in the (X|HT)ML-ish text portion of JSX then none of the automatically inserted comments work and I manually have to type out the {} and comment characters.
AFAICT just about every code editor has a feature like this. And they all won't work in JSX, and it would be unreasonable to put the burden of "fixing" this issue on them when it's the – not part of the standard defining .js files – JSX that created this issue.
Oftentimes you actually need comments emitted in HTML.
Such as adding notices into the source, debugging crazy off-by-one errors, inserting markers in the output for post-processing.
JSX is syntax used across multiple libraries (it's not only React) in several environments that are not the dom (most of my JSX work lately has actually be in React Native where there is no DOM; there are also other react renderers – a three.js canvas renderer, Blessed terminal interfaces, Titanium mobile apps, x11 windows, PDFs, Ionize/Electron apps, ...; I'm actually surprised no-one has made an "jsx2xml/ReactXML renderer" yet). I don't think we should add HTML comment nodes to the JSX syntax itself, especially when the actual use for explicit comment nodes is very small.
However, for when you do explicitly want a comment inside your html I do think it would be reasonable for react-dom to support a Comment element instead of forcing use of dangerouslySetInnerHTML (although it is still fair to note that comments are still inherently dangerous HTML thanks to IE).
import {Comment} from 'react-dom';
<div>
<Comment text='here is my comment' />
</div>
