pugjs / js-stringify

Stringify an object so it can be safely inlined in JavaScript code
MIT License
19 stars 4 forks source link

Array empty element converts to null #6

Open OwnageIsMagic opened 6 years ago

OwnageIsMagic commented 6 years ago
stringify(new Array(3)) /* "[null,null,null]" */
[,,][0] === null /* false */
ForbesLindesay commented 6 years ago

I would accept a pull request to fix this, but it requires us to manually do the stringifying of Arrays and Objects. That would let us properly handle undefined and Dates even when they are properties of objects/elements of arrays.

OwnageIsMagic commented 6 years ago

Is it really needed?

ForbesLindesay commented 6 years ago

I'm confused. This was your issue. If you don't think the problem needs solving, why did you open the issue?

OwnageIsMagic commented 6 years ago

Just to state that issues exists I don't know how this package is used I think that people expects that this package does 1:1 conversion, but this not true for this case https://github.com/pugjs/js-stringify/network/dependents?dependent_type=REPOSITORY states that this package used by 17k projects degrading performance for 17k projs for such edge case is somewhat not a good idea

OwnageIsMagic commented 6 years ago

btw fix is quite easy

if (Array.isArray(obj))
  return '[' + obj.map(x => stringify(x)).join(',') + ']';

but it will sometimes add +1 to arr length based on trailing comma support of runtime

ForbesLindesay commented 6 years ago

That example is wrong in a more subtle way:

[,,,,]
// => [empty x 4]
eval('[' + [,,,,].join(',') + ']')
// => [empty x 3]

I don't know whether this is super important to fix or not. I suspect not as sparse arrays are a pretty rare occurrence, but it would be nice to support undefined in arrays and Date objects in deeply nested locations.

OwnageIsMagic commented 6 years ago

Yes, as mentioned before trailing comma consumes last delimetr Here is easy fix for that one :) "engine": { "node": "<6" } Accesing out of bounds will return undefined, so no problem except array.length But code like this is already broken by Node (and this change will be semver major, so its ok)