simone-sanfratello / json-stringify-extended

JSON.stringify any data types
17 stars 1 forks source link

compress doesn't work with functions #2

Closed haifeng239 closed 6 years ago

haifeng239 commented 6 years ago

example

var stringify = require('json-stringify-extended');
var obj = {foo: function(){return true}};
console.log(stringify(obj, {compress: true}));

result

{
  foo:undefined
}

the following will fix it, but I'm hoping there's a nicer option. I couldn't find any relevant uglify options to support direct minification of "function(){}"

var uglify = require('uglify-js');
var input, output, obj;

obj = {foo: function(){return true}};
input = "var ___ = " + obj.foo.toString();
output = uglify.minify(input).code
output = output.replace('var ___=', '');
output = output.replace(/;$/, '');
console.log(input);
console.log(output);
simone-sanfratello commented 6 years ago

Oh, that's odd ... I'll working on it Thanks again for reporting bugs!

simone-sanfratello commented 6 years ago

I've just fixed in 1.0.1 with a solution similar to yours, please check if it works for you

haifeng239 commented 6 years ago

thanks for the quick turnaround. basic cases are working, however I have a special case that still fails.

const stringify = require('json-stringify-extended');

var fn = function(){};
fn.toString = function(){return "'test'";};
var obj = {a: fn, b: fn};

console.log(stringify(obj, {compress: true}));

output

{
  a:"test";,
  b:"test";
}
haifeng239 commented 6 years ago
const stringify = require('json-stringify-extended');
var fn = function(){}
fn.toString = function() {return "fn"}
var obj = {a: fn, b: fn};

console.log(stringify(obj, {compress: true}));
{
  a:fn;,
  b:fn;
}

with compress: false

{
  a:fn,
  b:fn
}
haifeng239 commented 6 years ago

works with the following addition. I think uglify output always ends with a semicolon, if not we could use regex instead.

          if (_min.code) {
            _min.code = _min.code.substr(0, _min.code.length - 1)
          }


var fn = function(){}
fn.toString = function() {return "fn"}
var obj = {"a": fn, "b": fn};

console.log(stringify(obj, {
  compress: true,
  endline: '',
  spacing: '',
  valueQuote: "'"
}));
{a:fn,b:fn}
simone-sanfratello commented 6 years ago

Ok, I trim trailing dot-commas with regexp, it's safer

However, that's not formally correct and does not behave as expected

  const _fn = function(){ return 0}
  _fn.toString = 1 // breaks pair between function and it's content
  console.log(_fn ,_fn())

output

#<Function>
0
haifeng239 commented 6 years ago

just curious, which version of node.js are you using? the output I see for that is

{ [Function: _fn] toString: 1 } 0

also, toString should always be a function (which ideally returns a string)

const _fn = function() { return 0; }
_fn.toString = function() { return 1;}
console.log('' + _fn)

the following would throw an error

const _fn = function() { return 0; }
_fn.toString = 1;
console.log('' + _fn)
haifeng239 commented 6 years ago

Thanks for the updates btw. This is a great project.

The original test case is still failing

const stringify = require('json-stringify-extended');

var fn = function(){};
fn.toString = function(){return "'test'";};
var obj = {a: fn, b: fn};

console.log(stringify(obj, {compress: true}));

I believe the try should look like this

          _min = uglify.minify(obj.toString(), FUNCTION_COMPRESS_OPTIONS)
          console.log(_min);
          if (!_min.code) {
            _min = uglify.minify(FUNCTION_COMPRESS_NAMED + obj.toString(), FUNCTION_COMPRESS_OPTIONS)
            _min.code = _min.code.substr(FUNCTION_COMPRESS_NAMED_LENGTH)
          }
          _min.code = _min.code.replace(/;+$/, '')
          return _min.code || obj.toString()
simone-sanfratello commented 6 years ago

Oh, sorry, I'll fix. I'm running node.js 8, current LTS. I don't agree to replace function toString() method, it's tricky and harmful, btw I think now it works properly, I mean, the type of the variable is a function but toString() doesn't return the function body