-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
don't include semi in JSX only code blocks in markdown #3196
Comments
I think this is a little more general. Do we want to detect when the code snippet is a single expression (i.e. not multiple statements), then omit semicolon accordingly? |
I guess the question would be if there are any situations where a user writes a single expression for which omitting the semicolon would be undesirable. I can't think of any off the top of my head. |
So do we have any conclusion here? Is this something you’d like to change? For both semi / no semi options? |
I believe it's something we want, but there's no one actively working on it to flesh out the details on this behavior (and of course, implement it later). |
Maybe this should be framed as a bug? Namely, when using no-semi, we shouldn't put a defensive prefixing semicolon in front of the first statement in the program. I guess that could break stuff for users who are concatenating scripts together, though... So maybe it should be: But it'd be kinda annoying if you had single-JSX-element blocks interspersed with normal code blocks, and you wanted to use semicolons in the normal code blocks. Because you'd need no-semi just for the JSX element blocks, but you'd want it elsewhere... So maybe we should special-case "markdown block whose program contains only one statement which is an ExpressionStatement whose expression is a JSXElement" specifically |
@suchipi This isn't specific to the first statement of a program; it applies to any statement starting with JSX. And it's not about concatenating scripts, it's about letting people move the line, or add new lines, without having to think about ASI. See #1433 (comment). I do understand that it looks strange in documentation, though. It's a very tricky case, with no obvious solution. I guess we'll find one after some more discussion, though. |
I think we may have gotten stuck a bit too much on the leading semi-colon, though. If you re-read this bit of the OP:
… you'll see that the actual request is to get rid of the semicolon regardless of the what the semi option is set to. In other words, the markdown code block is intended to show off just an expression, not a whole program. I guess the OP would want this expression without semicolon as well: ```js
1 + 2
``` However, Prettier can't know the intention – did the user intend to show just an expression or a whole program? We could simply omit semicolons when there's just a single expression in a markdown code bloc, but then I'd bet we'd get issue reports about missing semicolons in those cases. |
@lydell I meant that we could treat "first statement" differently than other statements, so it would be a bug in the "first statement" case. I don't think that's the right approach after thinking it over, though. Good to know about the editor story for moving lines around; hadn't thought about that. Since this issue is about improving the experience when using react-styleguidist, I think the right approach is to omit semicolons when there's a single JSX element expression in code blocks (but not other kinds of expressions). |
To clarify- let's remove semicolons regardless of the semi option in that case, per @lydell |
Spec: Don’t output semicolon before or after a single ExpressionStatement inside a markdown code block, regardless of the I think that will solve the problem. And if somebody complains about a missing semi-colon in that case, we’ll deal with that then. That’s a less worse problem to have than this issue. |
A workaround for now could be to mark the code blocks as json: ```json
<div>test</test>
``` That probably has other side effects, though. |
Why not: Don't output semicolon before or after a single [ExpressionStatement whose expression is a JSXElement] inside a markdown code block, regardless of the semi option. That would reduce the scope for the potential future "somebody complains about a missing semi-colon" case, and would solve this issue. |
Yeah, that's also an option. Not sure what's best. |
I’ve tried to implement it (#3330) but I need some help with detecting Markdown. |
I'm using prettier with a project that uses react styleguidist to document components (docs here: https://react-styleguidist.js.org/docs/documenting.html)
when including just JSX in a codeblock it would be nice if prettier would exclude the leading (or following depending on your
--no-semi
setting) from the outputPrettier 1.8.1
Playground link
Input:
Output:
Expected behavior:
The text was updated successfully, but these errors were encountered: