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

Please unlock issue #1080 (Provide way to use single quotes in jsx) #3437

Closed
iansinnott opened this issue Dec 7, 2017 · 24 comments
Closed
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.

Comments

@iansinnott
Copy link

#1080 was locked a while back by @rattrayalex. I totally understand not wanting to be bombarded with new issue notifications as a maintainer, but I'm suggesting it be unlocked because:

  • Users of Prettier who have found new workarounds for the issue can't share them with everyone else
  • The issue may get more 👍 / 👎 which provides passive information on the community's interest, without generating noise
  • The issue is marked as "help wanted" so anyone who is not yet a contributor might want to ask questions about what direction to take in making a PR

Even if that issue gets marked as a wontfix there's still conversation to be had that can benefit the wider community.

@rattrayalex
Copy link
Collaborator

rattrayalex commented Dec 7, 2017 via email

@iansinnott
Copy link
Author

Hey @rattrayalex thanks for the quick response. There was already a PR by @trotzig (#1084) made for this but it was closed down before you reopened #1080. So let's reopen that PR? 😄

The workarounds are for users in the meantime before a decision is reached on whether or not to include this quotes functionality. And if it's eventually decided not to include this feature then people will need the workarounds.

@josephfrazier josephfrazier changed the title Please unlock issue #1080 Please unlock issue #1080 (Provide way to use single quotes in jsx) Dec 7, 2017
@josephfrazier
Copy link
Collaborator

@iansinnott another option is to port --jsx-single-quote from prettier-miscellaneous

@azz
Copy link
Member

azz commented Dec 7, 2017

Don't think it's a good idea to add this. I can't think of a single reason why single is better than double quote beyond personal preference, which Prettier is built to eliminate. #1080 (comment) #1080 (comment)

@iansinnott
Copy link
Author

Good point @josephfrazier (and thanks for updating the title). Yeah that option works great in prettier-miscellaneous.

@azz Prettier was built for such a purpose but the project does currently support options based solely on personal preference (semicolon, quotes, bracket spacing, etc) so it seems there's precedent for such a thing. But more importantly configuration like this would helps to avoid fragmentation of the community (like the aforementioned prettier-miscellaneous).

Anyway, my purpose in opening this issue was not to reopen discussion in a separate place, but rather to allow the discussion to continue on the original issue and come to a conclusion, whichever way it goes.

@duailibe
Copy link
Member

duailibe commented Dec 8, 2017

@iansinnott I personally don't like the argument of "other options exist" very much.. if we follow that we can make a case for every single style option people suggest.

To be fair, some of the options exist for historical reasons and I believe they were likely to be denied if they were suggested today.

@azz
Copy link
Member

azz commented Dec 9, 2017

the project does currently support options based solely on personal preference (semicolon, quotes, bracket spacing, etc)

Post hoc ergo propter hoc. Two of the three options you mentioned, along with tab-width, print-width and trailing-comma, have been in Prettier since the beginning, pretty much inherited from recast (of which Prettier is a fork). They should not be considered "precedent" because they were never really "requested" so much as they were a part of the original documented API.

The semicolon option (requested way back in in #12) was added to allow a huge number of people following styles such as standard to adopt Prettier. I really doubt changing " to ' for JSX falls into that bucket.

@rattrayalex
Copy link
Collaborator

Sorry, @iansinnott , I would personally be open to reopening #1084 but judging by reactions from other maintainers on this thread it would be closed again. I'll close this if that's okay.

It seems prettier-miscellaneous supports the feature you're after, so if this is a deal-breaker for you, I'd suggest going there instead.

@amsul
Copy link

amsul commented Dec 12, 2017

As much as I love prettier, I still find this to be a really odd default convention.

If the config specifies singleQuote, why would JSX not respect that? Especially when you consider that JSX !== "HTML in JS".

Kinda unfortunate that a library that's supposed to be used to ensure "consistent style" has this form of inconsistency within the same file..and the suggested solution is to use a fork instead of fixing it in the actual library itself.

@duailibe
Copy link
Member

duailibe commented Dec 12, 2017

@amsul I'm sorry you find that odd, but the "consistent" is supposed to mean that, given an AST, it will always print your code the same way, regardless of your original code; it's not supposed to mean that we'll print similar things the same way through your code (though that's generally what happens).

That means Prettier will always print strings in JSX attributes with double quotes and the rest as you specified. This might seem silly but Prettier applies some normalizations your non-JSX strings (e.g. remove unnecessary escapes) while if you did those in JSX it would break your code.

@iansinnott
Copy link
Author

Thanks for taking the time to respond everyone. I'm not trying to change anyone's mind on this. But #1080 is still locked, so those of us who have already expressed interest in this issue are prevented from sharing solutions with each other and anyone else who might come across the issue.

@rattrayalex
Copy link
Collaborator

@iansinnott do you have a solution to share?

@amsul
Copy link

amsul commented Dec 13, 2017

I'd just like to say, I realize the goal of Prettier is quite ambitious..and would essentially stop these kinds of discussions from ever happening (which is a great goal!)

However, I'm curious about what the maintainers think is considered "in scope" of what Prettier will and won't tackle.

Considering JSX is just sugar on top of JS, why should Prettier bother applying extra semantics (and exceptions to rules) for it by adding a unique way to handle strings? In my opinion, that is an additional configuration which just cannot be "unconfigured".

In fact, a configuration that actually seems to spawn from trying to bring XML conventions into JS.

Does that mean these silly discussions will pop up every time the community introduces new syntactic sugar on top of JS that bring conventions from other languages?

@duailibe
Copy link
Member

Hi @amsul!

For Prettier, I don't think it matters if JSX is sugar or not. We only care about the syntax, which is spec'ed at http://github.com/facebook/jsx and that syntax requires extra semantics (which I have just showed above).

@amsul
Copy link

amsul commented Dec 13, 2017

@duailibe I understand your point about the extra semantics required for normalization - which is simply resolved by adding {}.. but that's independent of ' vs ".

Anyways, I've said my final thoughts above - I hope it registers with people enough to fix this!

@duailibe I appreciate you taking time to give responses 🙏

@iansinnott
Copy link
Author

@rattrayalex yup (run prettier output through string replacement).

But even if I can share this solution it doesn't help anyone else share theirs (or improve on existing ones). That's why I suggest unlocking the original issue.

Also, perhaps the issue should be closed if the prettier community decides against single quotes in JSX. Either way though, keeping it locked makes it difficult for those of us that care about this to help each other.

@ljharb
Copy link

ljharb commented Jan 24, 2018

Can’t comment on the original issue, but huge 👎 for this - jsx is html, html has double quotes, and JS has single quotes. Please do not change the default behavior.

@j-f1
Copy link
Member

j-f1 commented Jan 24, 2018

jsx is html

Not really:

// This references `Component`
<Component notANumber="2" invalid=100% yesYouCan=<DoThis /> {...notHTML}>
  <!-- this is not a comment -->
  a
  {/* no space rendered here */}
  b
</Component>

@ljharb
Copy link

ljharb commented Jan 24, 2018

Conceptually, it is, even though it follows slightly different semantic rules. It’s certainly not JS, by any metric, so “consistency between JSX and JS” is a brittle strawman.

@j-f1
Copy link
Member

j-f1 commented Jan 24, 2018

@ljharb: @gaearon echoes my thoughts very well in airbnb/javascript#269 (comment).

@ljharb
Copy link

ljharb commented Jan 24, 2018

Fair. But all those confusions are or can be addressed by linter rules (ones the airbnb config enforces), which is a much more reliable signal than “it has single quotes”.

@gaearon
Copy link

gaearon commented Jan 24, 2018

FWIW I've long stopped caring about things like this and I can't recommend that enough

@georgeawwad
Copy link

I for one welcome our new double quote JSX overlords

@Jessidhia
Copy link

Jessidhia commented Feb 19, 2018

@azz Users of standard absolutely have single quotes in their JSX. The rule there still is to use single quotes unless you need to write an ', at which point you use double quotes.

(This currently is probably the main blocker to adopting prettier as-is at work. We don't use english prose in our strings so we almost never encounter the apostrophe problem.)

nottobe pushed a commit to mesaic/eslint-config-mesaic that referenced this issue Mar 2, 2018
prettier does not support jsx-quotes and always will use double-quotes even
when the option --single-quote is enabled. So prettier will change all quotes
in jsx to double quotes and eslint will change it back to single quotes.
prettier/prettier#3437
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jul 5, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.
Projects
None yet
Development

No branches or pull requests