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

Beautify JSON files #156

Closed
RefinedSoftwareLLC opened this issue Mar 22, 2013 · 19 comments
Closed

Beautify JSON files #156

RefinedSoftwareLLC opened this issue Mar 22, 2013 · 19 comments

Comments

@RefinedSoftwareLLC
Copy link

So my third rewrite and many edits later...

Currently my project uses files.json. Some store current settings while others store a listing of all possible options for one setting. For example, every server's name and ip address are stored in serverList.json that is updated automatically every minute. I would like to use UglifyJS2 for this. The following code is my default but doesn't have any of your advanced features like max line length and looks horrible:

var data = JSON.stringify(json_object, null, 4);

Currently the following code gets me working results:

var data = JSON.stringify(json_object);
data = UglifyJS.minify(data, {
fromString: true,
compress: false,
output: {
beautify: true,
quote_keys: true
}
}).code;
data = data.substr(0, data.length - 1);

But could you add .json beautify support, maybe like this:

var data = UglifyJS.minify(json_object_or_json_uglified_string, {
fromJSON: true,
output: {
beautify: true,
}
}).code;

and / or a non-standard shortcut like:

var data = UglifyJS.beautifyJSON(json_object_or_json_uglified_string, { no_required: configs } );

This code would have "compress: false" (or whatever part of compress is causing an error and any parts that could cause data loss as even unused properties should not be removed), "quote_keys: true", no trailing semicolon, and auto detect between json_object and string. If it gets passed an json_object it automatically runs JSON.stringify().

If you add "fromJSON: true" you could then add any custom formatting you wanted to beautifying JSON files like not having the file end with a semicolon (which crashes JSON.parse(), "semicolons: false" did not fix this issue). Not having double quotes on the keys broke JSON.parse() in nodejs. The retrieve function that must not error out is:

var json_object = JSON.parse(data);

@tonylukasavage
Copy link

I hacked v1 to do the same. Don't forget to make sure large numbers don't use exponential format (10e23) as that will break JSON as well.

@michaelficarra
Copy link
Contributor

Passing the output of JSON.stringify to uglifyjs shouldn't even work. That's not a JavaScript program.

@curiousdannii
Copy link

Yes it is, it's a side-effect free string literal.

@RefinedSoftwareLLC
Copy link
Author

From my understanding JavaScript is a super set of JSON. Meaning that JSON is always valid JavaScript code but JavaScript is not always valid JSON code (JSON does not store pointers, functions, Infinite, NaN, etc). In this case a trailing semicolon breaks JSON, (the only semicolon in the whole returned string).

@michaelficarra
Copy link
Contributor

No. JSON-serialised objects are never valid JavaScript. This is not a JavaScript program:

{"a": 0}

This is, but probably not for the reason you think:

{a: 0}

That's a block with a labeled ExpressionStatement that contains 0.

({a: 0})

And that's an object literal, identical to the JSON above. Have you never wondered why you have to put parentheses around JSON in order to eval it (treating it as JS)? It's because you need to put it in expression position. JSON is valid JavaScript if it's already in expression position.

@vendethiel
Copy link

This is, but probably not for the reason you think:

{a: 0}

Tel me if I'm wrong -- this is a block with a label in it, right :p ?

@RefinedSoftwareLLC
Copy link
Author

I am not using eval() to turn JSON strings to Objects that just store data. On Chrome I can do either of these just fine:

configBill = { a: 1, b: 2, c: 3 }
configJill = { "a": 1, "b": 2, "c": 3 }

for the same results, a data object. So whether in a JavaScript code file or in a JSON file, they can look exactly the same to non-programmers that are changing configurations or doing data entry.

@mishoo
Copy link
Owner

mishoo commented Mar 22, 2013

@Nami-Doc not exactly. It's a block containing one labelled statement. The statement is 0; and the label is a.

@michaelficarra
Copy link
Contributor

@Nami-Doc: correct.

@RefinedSoftwareLLC: 🤦 Alright, you're right, JSON is obviously a subset of JS because the almighty Chrome tells us it is so. incorrect.

edit: removed snarkiness

@vendethiel
Copy link

@Nami-Doc not exactly. It's a block containing one labelled statement. The statement is 0; and the label is a

That's what I meant ^^

I am not using eval() to turn JSON strings to Objects that just store data. On Chrome I can do either of these just fine:

configBill = { a: 1, b: 2, c: 3 }
configJill = { "a": 1, "b": 2, "c": 3 }

for the same results, a data object. So whether in a JavaScript code file or in a JSON file, they can look exactly the same to non-programmers that are changing configurations or doing data entry.

re-read what @michaelficarra said.
{} on toplevel is a bloc. An assign is not toplevel. Try yourself in chrome.

> {}
undefined
> {a: 1}
1
> ({a: 1})
Object {a: 1}

@mishoo
Copy link
Owner

mishoo commented Mar 22, 2013

So, as @michaelficarra points out, a JSON expression is not always a valid JavaScript program (sometimes it is, for example this JSON is also valid JS: [ "foo", 1, 2 ]). This said, we could indeed provide a CLI option to beautify JSON. So I'm leaving this open, maybe I'll do sometime.

@RefinedSoftwareLLC
Copy link
Author

Well I am using NodeJS, which runs on Chrome V8, I haven't tested on other browsers. But I am not planing on having my clients changing settings in a JSON file in their HTML5 cache folder either. The following code seams quite useless as it doesn't do anything but take up CPU and Memory:

{"a": 1}

Where as the following is useful:

myVar = {"a": 1}

So I do not get why JSON not working in the toplevel is an issue because JSON in the toplevel is completely useless because it doesn't have any functions only properties.

@michaelficarra
Copy link
Contributor

So I do not get why JSON not working in the toplevel is an issue because JSON in the toplevel is completely useless

@RefinedSoftwareLLC: Then why are you trying to parse it that way? Because you find some use in it. Unfortunately for you, your JSON is not valid JS, and can only be working now due to a bug in the UglifyJS parser.

edit: elaborated

@RefinedSoftwareLLC
Copy link
Author

This is how a JSON file looks in a text file or as the string of a JSON text file:

{"a": 1}

This is how you copy and paste this by hand into a JavaScript file:

myVar = {"a": 1}

To programmatically do this (like my config files) you use the following:
/* text file looks like this:
{"a": 1}
*/
data = readFile();
myVar = JSON.parse(data); // data = "{"a": 1}"

I would like to use UglifyJS2 to change the JSON text file from:

[{"a":1,"b":2},{"a":1,"b":2}]

to a readable form that is better then the other Beautifiers out there:

[ {
"a": 1,
"b": 2
}, {
"a": 1,
"b": 2
} ]

@michaelficarra
Copy link
Contributor

@RefinedSoftwareLLC: I think you're confusing function calls with macros. Function calls work with data structures. Macros with with strings of program text. When you call JSON.parse, it doesn't put a string of program text in that position literally.

We now see why UglifyJS was accepting your JSON as JS: you are serialising an array, which will produce perfectly valid JS.

Just use JSON.stringify. It does a good job and works with the kind of data you want.

JSON.stringify(JSON.parse(your_json), null, 2);

edit: If you really need the max line length, use UglifyJS like below so it is safe.

var data = JSON.stringify(json_object);
data = UglifyJS.minify('(' + data + ')', {
  fromString: true,
  compress: false,
  output: {
    beautify: true,
    quote_keys: true
  }
}).code;
data = data.replace(/\(\s*(.*)\s*\);?/, '$1');

@RefinedSoftwareLLC
Copy link
Author

Yes, I just checked using my hack above and it doesn't work if the top level is not an array.

Error
at new JS_Parse_Error (C:\projects\webstorm\mother\node_modules\uglify-js\lib\parse.js:185:18)
at js_error (C:\projects\webstorm\mother\node_modules\uglify-js\lib\parse.js:199:11)
at croak (C:\projects\webstorm\mother\node_modules\uglify-js\lib\parse.js:630:9)
at token_error (C:\projects\webstorm\mother\node_modules\uglify-js\lib\parse.js:638:9)
at unexpected (C:\projects\webstorm\mother\node_modules\uglify-js\lib\parse.js:644:9)
at semicolon (C:\projects\webstorm\mother\node_modules\uglify-js\lib\parse.js:664:43)
at simple_statement (C:\projects\webstorm\mother\node_modules\uglify-js\lib\parse.js:822:73)
at C:\projects\webstorm\mother\node_modules\uglify-js\lib\parse.js:693:47
at C:\projects\webstorm\mother\node_modules\uglify-js\lib\parse.js:677:24
at block_ (C:\projects\webstorm\mother\node_modules\uglify-js\lib\parse.js:938:20)

I guess I can use:

if (typeof data === "string") {
JSON.stringify(JSON.parse(data), null, 4);
} else {
JSON.stringify(data, null, 4);
};

But once again this does not beautify as well as UglifiyJS2 nor have advanced features like max line length.

Does anyone know a hack to fix the above error? maybe adding "[" and "]" to the beginning and end of the string then removing them after...

@michaelficarra
Copy link
Contributor

@RefinedSoftwareLLC: see my edit above.

@RefinedSoftwareLLC
Copy link
Author

#158

Okay I just requested a pull of adding my feature:

UglifyJS.beautifyJSON(files, options) { return beautifiedString; };

to the master branch. I just spent all day working on it. I tried to keep it similar to your code while still keeping it readable and easy for me to follow. The biggest difference is that it supports giving it Objects and arrays of objects; and using JSON.stringifies replacer feature. What do people think of this feature? I have already started using this like so:

console.log(UglifyJS.beautifyJSON(object, { fromObject:true } ));

I wish that the option fromFile:true wasn't the default (if it existed as it is currently implied), I kept this the default behavior though for consistency.

@mishoo
Copy link
Owner

mishoo commented Mar 23, 2013

@RefinedSoftwareLLC I'm sorry, looks like I misunderstood this ticket. In fact, looking at the patch I can hardly figure out what it is that you need to do. I thought you needed to parse JSON, but that's not the case since you don't touch the parser. Your code would possibly work only for arrays, not when the JSON is an object literal.

Either way, it seems it's something that can be done outside UglifyJS, and it's an isolate case that might be useful to you but not to anyone else out there. So please do it in your code, not in UglifyJS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants