Keep string names in object literal#1108
Keep string names in object literal#1108yamafaktory wants to merge 6 commits intoprettier:masterfrom
Conversation
The printProperty helper function was implemented as a workaround to an existing Flow bug which no longer exists. Every function call can be simply replaced by a path.call() invocation.
|
Heads up, there are merge conflicts on this PR! (I don't think Github notifies users if their PR gets blocked by this.) |
…103-keep-string-names-in-object-literal
|
@ericclemmons Sadly no notification for a merge conflict... but this should be fine now 😄. |
|
I thought we did this on purpose (remove strings when they aren't necessary)? |
|
If this is merged, I'd like an option for it. Not everyone (few, I'd wager) needs closure compiler optimizations |
|
This initial issue mentioned both Google Closure Compiler & Uglify. IMHO, if you voluntarily stringify object literals' keys in your codebase, this should be kept. |
Why should quoted keys be treated differently than other stuff like quotes, semis, linebreaks etc? Prettier's great strength (IMO) is enforcing consistency in all things by not (at least rarely) caring about input formatting at all. There should be strong arguments to change away from that, and while advanced optimizations is a strong argument, I do think most people (e.g. close to 100% of people writing for Node) would prefer the current behavior. So it's better to add an option. |
|
As the initial issue highlighted it because this simple modification has consequences on how some tools are going to interpret it. You can see that difference in the AST, so why removing it? I don't see that the same way as switching e.g. from double to single quotes. But this is just my opinion. |
|
@yamafaktory I think it's a major deficiency of ESTree that it represents |
|
@vjeux / @jlongster I've synced it with current master and updated the tests accordingly. Please decide on whether or not this is an improvement (thinking of Google Closure, etc) or a regression as it involves some debatable inconsistencies 👍. |
|
So sorry I've been letting this one hang for a long time. I don't think we should do that. In practice, the number of people that managed to use the Google Closure Compiler with Advanced Mode is extremely tiny and I feel like it's better to normalize those keys. |
|
The concerns raised in this make a lot of sense, but I thought I'd chime in with a similar but alternate use case. In this case, we'd like the However, prettier will automatically strip out these single quotes, and I'd prefer not to litter prettier ignore flags all over our code base. We're going to accept what prettier says for now, but I thought I'd raise this in case other people felt similarly. |
|
I ran into this myself. It conflicts with one of my TSLint rules that requires consistent quoting in objects literals, and I think it's fair for prettier to go with this convention as well, to denote that if an object requires quotes for keys, that it's probably being used as a dictionary as opposed to an object with properties. |
|
@yamafaktory Did you think about trying to merge into https://github.com/arijs/prettier-miscellaneous ? |
|
@kushal feel free to reuse the code from this PR if you plan or wish to have that feature in the fork you mentioned. |
This one should fix #1103.