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

Allow leading comma and empty values in arrays #19

Closed
aseemk opened this issue Jun 3, 2012 · 10 comments
Closed

Allow leading comma and empty values in arrays #19

aseemk opened this issue Jun 3, 2012 · 10 comments

Comments

@aseemk
Copy link
Member

aseemk commented Jun 3, 2012

Edit: the original title of this case was to disallow these things, considered bugs. That was fixed, but now the conversation below asks to consider re-allowing them.

ES5 allows these:

[,]     // => [undefined]
[,null] // => [undefined, null]

But I don't think that's needed in JSON5. Let's keep it simple and closer to JSON here.

This implementation currently allows these (by design); let's fix.

@williamkapke
Copy link

I'm in favor of allowing this. Reason being is that there is no other way to describe "nth item of Array" except for having null (or empty) spots.

For instance; Say I am doing some validation on an array that I want to only have numbers in:

[
  45,
  18,
  "eleventeen",
  65535,
  "the end"
]

I'd like to respond with something like:

[
  ,
  ,
  "Numbers only please",
  ,
  "Numbers only please"
]

I asked Crockford why he didn't allow this (it has been allowed in JS for a VERY long time) and he said it was because not all languages handle sparse arrays.

Because javascript doesn't allocate based on the length of an array- it is easy to have a seemingly huge array:

var x = [];
x.length = 4294967295; //Chrome's limit

Indeed, it could be trouble for a parser/stringifier to do a lot of work in other languages to handle this. However- it seems like a lame argument to me since it would sill have the problem if all the values were NOT null.

@aseemk
Copy link
Member Author

aseemk commented Jul 27, 2013

@williamwicks: Are you saying you specifically want the array to contain undefined elements? The undefined type itself is not part of JSON. You can return null elements though even in regular JSON:

JSON.stringify([,,,])
# "[null,null,null]"

Does that work fine for you?

@williamkapke
Copy link

Are you saying you specifically want the array to contain undefined elements?

Yes. It seems wrong for serialization to change the representation of the object. For example:

var original = [,,,];
var stringified = JSON.stringify(original); // "[null,null,null]"
var parsed = JSON.parse(stringified);

assert.strictEqual(parsed[1], original[1])
// AssertionError: null === "undefined"

Using null place holders is an improvement, but I'd rather see it just be empty. I went back to your README which seems to be a sort of "mission statement" for this project. It says:

It remains a strict subset of JavaScript, adds no new data types, and is a strict superset of existing JSON

That being the case, it seems this should be acceptable.

It is the only representation that allows it to be stringify'ed & parsed back while maintaining the original representation.

One other thing to point out to anyone that comes along and reads this: To hold true to how Javascript handles[,,,] means that the parser needs to count the last one as a "trailing comma" and not increment the length.

[,,,].length; // 3
[1,2,3,].length; // 3
[1,2,,4].length; // 4

Thanks for considering it!

@aseemk
Copy link
Member Author

aseemk commented Jul 28, 2013

That's a good point about the representation changing, but ultimately I think what this boils down to is adding the notion of undefined to JSON5. That's probably unlikely to happen, unfortunately, since most languages don't have a concept for it vs. null.

(This also fits the "adds no new data types" part of the mission statement.)

And without undefined, supporting sparse arrays using null doesn't seem like a good idea, either, since that'd differ from the semantics of JS.

So ultimately it seems like your best bet is to explicitly include null elements in your array. WDYT?

@williamkapke
Copy link

[,,,] is just a syntax (I call them "empties"). How that syntax is handled can be left up to the language specific implementations. It does not require JSON5 to add an undefined type. JSON5 could just say that it is "empty"- ignore it if you want.

But I suspect it would most likely end up like this...
Languages without undefined:

  • Will treat "empties" it as nulls.
  • Consumers of the array will see no difference.
  • Implementations may use "empties" as a compact form of [null,null,null].

Languages that do have undefined

  • Will treat "empties" as undefineds and nulls as nulls.
  • Consumers of the array will understand that null and undefined are not guaranteed across implementations.
  • Implementations will use "empties" only for undefined values when serializing.

Yes- explicitly injecting nulls does the trick for retaining item indexes. I've been doing it that way for years. If we're going to improve JSON- this is my feature request. Will it break anything or limit usage?

@jasonswearingen
Copy link

as @william mentions, i think care has to be taken for breaking
data-round-trip with JSON. JSON5-->obj-->JSON3-->obj. both "obj"
data structures should be identical. if any changes to JSON5 should
ensure that the round-trip is still valid.

  • Jason

Jason Swearingen Technical Director: Novaleaf Softwarehttp://www.Novaleaf.com

JasonS@Novaleaf.com Cell: +66-089-775-0111

Office: +66-02-234-3031 Fax: +66-02-234-3032

On Mon, Jul 29, 2013 at 3:20 AM, William Wicks notifications@github.comwrote:

[,,,] is just a syntax (I call them "empties"). How that syntax is
handled can be left up to the language specific implementations. It does
not
require JSON5 to add an undefined type. JSON5 could just say that it
is "empty"- ignore it if you want.

But I suspect it would most likely end up like this...
Languages without undefined:

  • Will treat "empties" it as nulls.
  • Consumers of the array will see no difference.
  • Implementations may use "empties" as a compact form of
    [null,null,null].

Languages that do have undefined

  • Will treat "empties" as undefineds and nulls as nulls.
  • Consumers of the array will understand that null and undefined are
    not guaranteed across implementations.
  • Implementations will use "empties" only for undefined values when
    serializing.

Yes- explicitly injecting nulls does the trick for retaining item
indexes. I've been doing it that way for years. If we're going to improve
JSON- this is my feature request. Will it break anything or limit usage?


Reply to this email directly or view it on GitHubhttps://github.com//issues/19#issuecomment-21690210
.

@aseemk
Copy link
Member Author

aseemk commented Jan 17, 2014

Lemme re-open this case for consideration.

@jordanbtucker
Copy link
Member

I move to continue disallowing leading commas and empty values. And I apologize for the following novel. There's a TL;DR at the bottom. :)

One of the headaches of JSON is that it doesn't allow trailing commas. For instance, you start to write a JSON file, and like any normal JavaScript developer, you add a comma after each value because it makes it easier to copy and paste and move values around.

{
  "end_of_line": "lf",
  "indent_style": "space",
  "indent_size": 4,
}

Of course, JSON throws SyntaxError: Unexpected token } because of the trailing comma. This has led to hacks like adding commas to the beginning of each value except the first.

{
  "end_of_line": "lf"
  ,"indent_style": "space"
  ,"indent_size": 4
}

It gets the job done, but it's ugly IMO, and it creates another problem. If you remove the first value and forget to remove the proceeding comma, you get SyntaxError: Unexpected token ,.

Fast forward to today. You start using JSON5 because trailing commas are awesome and should have been allowed by JSON in the first place. Even better, you don't have to change your current JSON files because JSON5 is completely backward compatible. And that's great because you have a lot of JSON files using the hacky leading comma syntax.

You edit one of your old JSON files, removing the first value, and you forget to also remove the proceeding comma because you're involved in several projects, overworked, and having trouble paying attention to detail.

But that's okay because JSON5 throws SyntaxError: Unexpected token , when you try to parse it. Because you've seen it so many times with JSON files, you immediately know the issue.

Actually, JSON5 throws SyntaxError: Bad identifier. Remind me to fix that. :)

TL;DR

If JSON5 allowed leading commas and empty values, old JSON files that used to throw would no longer do so, which could introduce silent and/or hard to find bugs.

Also, undefined is its own data type, and, repeating for emphasis, JSON5 introduces no new data types over JSON.

Thoughts?

@aseemk
Copy link
Member Author

aseemk commented Aug 3, 2014

Very interesting point @jordanbtucker! But I'm not sure I totally buy this argument:

If JSON5 allowed leading commas and empty values, old JSON files that used to throw would no longer do so, which could introduce silent and/or hard to find bugs.

Because there are plenty of examples where invalid JSON (that would throw) is valid JSON5 — e.g. unquoted keys, comments, even trailing commas as you mention. =)

I do agree that this still feels like it'd implicitly be adding a new data type. Your suggestion of leaving the element types unspecified/"empty" is an interesting one though, @williamwicks.

On the notion of round-tripping, @jasons-novaleaf, I'm still a bit unconvinced, because both native JSON and JSON3 fail the round-trip test here too. So this isn't new to JSON5.

At this point, I'm in agreement with @jordanbtucker that I don't think we should add this.

@jordanbtucker
Copy link
Member

Hey, everyone. I'm going to close this issue because it is incompatible with the official specification. If you would like to continue this discussion, please open an issue on the official specification repository. Please see the new issues template for more information. Thanks.

@json5 json5 locked as off-topic and limited conversation to collaborators Aug 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants