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

Change default value for quoteProps option to "preserve" for JSON5 #4639

Open
suchipi opened this issue Jun 5, 2018 · 10 comments
Open

Change default value for quoteProps option to "preserve" for JSON5 #4639

suchipi opened this issue Jun 5, 2018 · 10 comments
Labels
lang:json Issues affecting JSON status:needs discussion Issues needing discussion and a decision to be made before action can be taken type:enhancement A potential new feature to be added, or an improvement to how we print something
Milestone

Comments

@suchipi
Copy link
Member

suchipi commented Jun 5, 2018

Context:

#4636 (comment) by @suchipi:

@ikatyang based on this issue and #4611 I think we should not convert Object keys to identifiers by default in the json5 printer; it is surprising behavior to some. Thoughts?

#4636 (comment) by @ikatyang:

@suchipi I'm not sure what should we do here since converting object keys to identifiers is valid in JSON5, and we don't want to add options. And also if we're not going to convert them, there's no difference between JSON and JSON5, thus no need to split JSON5 from JSON.

And based on the response from linguist, we're probably going to add .jsonc to JSON's extensions for #4611.

(It's really weird to me why VSCode/Atom didn't have the built-in JSON5 support.)


Should we change the JSON5 printer to not convert object keys to identifiers?

I'm thinking yes, since JSON5 support in editors is hit or miss- for example, VS Code treats .babelrc as a JSON file instead of JSON5.

Personally, the only JSON5 feature I've ever leveraged was comments. But that's only one data point.

@suchipi suchipi added status:needs discussion Issues needing discussion and a decision to be made before action can be taken lang:json Issues affecting JSON labels Jun 5, 2018
@j-f1
Copy link
Member

j-f1 commented Jun 5, 2018

IMO we should keep it as-is and fix the bugs in editors.

@aerze
Copy link

aerze commented Jul 1, 2018

@suchipi I would like to put a vote in for keeping .babelrc as JSON, but not modify JSON5 printer.

It really is surprising/frustrating when the editor and the babel docs, here for example still default to "normal" JSON.

@bakkot
Copy link
Collaborator

bakkot commented Jan 9, 2019

When people write JSON5 (in a way which makes it invalid JSON), do they tend to omit the quotes? Is there a way that could be assessed?

@jordanbtucker
Copy link

When people write JSON5 (in a way which makes it invalid JSON), do they tend to omit the quotes? Is there a way that could be assessed?

Hi, I'm the current maintainer for JSON5. To answer your question, it's a bit of a mixed bag. The official JSON5 library outputs unquoted keys where possible, and single quoted otherwise.

When handwriting JSON5, I prefer to keep all of my keys unquoted if possible, but quote all of my keys if at least one of them must be quoted, just for consistency. I would say that most JSON5 documents are able to just use unquoted keys. (i.e. most keys in JSON and JSON5 documents are valid identifiers.)

My opinion would be to keep unquoted keys unquoted, and change the quote character for quoted keys according to the config file. For example, with the default config:

{ a: 1, 'b': 2, "c": 3 }

becomes

{ a: 1, "b": 2, "c": 3 }

@thorn0
Copy link
Member

thorn0 commented May 8, 2020

@jordanbtucker In other words, you propose that the default value for the --quote-props option should be changed to preserve (currently it's as-needed, same as for JS). Right?

Prettier 2.0.5
Playground link

--parser json5
--quote-props preserve

Input:

{ a: 1, 'b': 2, "c": 3 }

Output:

{ a: 1, "b": 2, "c": 3 }

@jordanbtucker
Copy link

@thorn0 I suppose so. Although I personally wouldn't want that to be the default for my JS files, haha.

@thorn0
Copy link
Member

thorn0 commented May 8, 2020

This issue was opened when the --quote-props option didn't exist. The option was added in April 2019 because it was needed for Closure Compiler. Now that we have it, this issue is probably not an issue anymore. People can use the option to adjust printing of JSON5 to their liking. Still, it might make sense to change this default (for JSON5 only, we aren't talking about JS here) and fix #5708 (comment) to make --parser json5 "just work" out of the box for VS Code's "JSON with comments".

@thorn0
Copy link
Member

thorn0 commented May 8, 2020

Changing the default would be a breaking change though. I'm not sure what exactly it can break in this specific case, but usually we consider such changes breaking and suppose that they need a major version bump. So it's probably not worth the trouble. Prettier has the option now, those who need it can use it.

@jordanbtucker
Copy link

Makes sense to me. Is there a way to mark this change to be added in the next major version?

@thorn0 thorn0 added this to the 3.0 milestone May 8, 2020
@thorn0 thorn0 added the type:enhancement A potential new feature to be added, or an improvement to how we print something label May 8, 2020
@thorn0 thorn0 changed the title Keep keys quoted in JSON5? Change default value for quoteProps option to "preserve" for JSON5 Feb 15, 2021
@fisker fisker modified the milestones: 3.0, 4.0 Feb 8, 2023
@fisker
Copy link
Sponsor Member

fisker commented Feb 8, 2023

I'm going to postpone this, since it's not been discussed for a long time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang:json Issues affecting JSON status:needs discussion Issues needing discussion and a decision to be made before action can be taken type:enhancement A potential new feature to be added, or an improvement to how we print something
Projects
None yet
Development

No branches or pull requests

7 participants