Disallow trailing commas in object literals entirely when targeting ES3#284
Disallow trailing commas in object literals entirely when targeting ES3#284DanielRosenwasser wants to merge 4 commits intomasterfrom
Conversation
This was done because trailing commas in object literals are not accepted by ES3. Fixes #271.
Also removed trailing commas from object literals in the compiler and certain tests.
DanielRosenwasser
changed the title
scripts/processDiagnosticMessages.ts
Outdated
There was a problem hiding this comment.
if (i > 0) {
result += ",\r\n";
}
|
👍 |
|
I do not think this is the right fix. we do not need to error on this. it is trivial enough to remove. please hold off on this change until we discuss it. |
|
I know, I'm just putting this change out there for the sake of discussion. I believe that the original fix (#273) supplies the right user experience, but in that case we need to amend the specification to reflect this. |
|
I believe the "correct" thing to do is error. That said, being correct is not always the "right" thing to do (esp. as we've shipped). I would simply document that TypeScript augments the ES3 grammar to add hte comma. |
|
If it was common practice in browsers to allow this for ES3, than it seems an unnecessarily strict check on our part. We would need to show that this isn't the case. Perhaps as a meta point, new restrictions should be done judiciously and should be giving tangible user benefit. I'm unclear on this point for this patch. |
|
👎 I don't like this fix. It actually breaks running the compiler with cscript.exe because cscript.exe only supports ES3. Also, I don't see why it is so important to preserve a trailing comma in an object literal as it has no semantic meaning. There are lots of other things we don't preserve in the emitter. For example, we always emit semicolons even if they weren't present in the input. |
|
Actually, this commit should be fine for cscript, but it's bad for the end user (and us, since we use trailing commas everywhere. I agree that it should not be merged. The problem is that if we don't do this, we no longer conform with the spec. What would be appropriate is if in the spec, we said we use ES5 parsing rules for trailing commas in object literals. |
|
Just to clear the confusion, this was an alternative fix for #271 that Daniel wanted to bring forward. I believe everyone agrees that this is a restriction that does not add value to the user, and omitting the trailing comma, though not inline with the sprit of staying close to the input is the right thing to do here. Daniel has already checked in a different change in #273. so closing this request. |
5 participants
The idea is that since the ECMAScript 3 spec does not permit trailing commas in an object literal, we should not permit the construct unless we have a revised grammar for TypeScript that says otherwise.
This is a breaking change. I would personally rather amend the specification to permit trailing commas and simply not preserve them.
This is an extension of the work on #273.