Conversation
|
TSLint's There's an option to forbid trailing commas in that position, but it's not enabled by default. That means this change will break a lot of projects. |
|
Not to put too fine a point on it, but we're not responsible for projects enforcing that they have a syntax error, even if we mistakenly didn't flag that error |
|
It looks like the same issue is present for array rest and object rest. also passes without error. |
|
And not only on binding patterns, but also in destructuring assignments: ({...foo,} = {});
[...bar,] = []; |
src/compiler/checker.ts
Outdated
| return grammarErrorOnNode(parameter.dotDotDotToken, Diagnostics.A_rest_parameter_must_be_last_in_a_parameter_list); | ||
| } | ||
| if (parameters.hasTrailingComma) { | ||
| return grammarErrorOnNode(parameter.dotDotDotToken, Diagnostics.A_rest_parameter_may_not_have_a_trailing_comma); |
There was a problem hiding this comment.
I would say that this is the wrong span to error on. Either error on the comma, or the parameter itself.
ghost
force-pushed
the
rest_param_trailing_comma
branch
from
3f3b9f3 to
f69d5b4
Compare
|
Noticed that tslint has a |
| } | ||
|
|
||
| function checkGrammarForDisallowedTrailingComma(list: NodeArray<Node>): boolean { | ||
| function checkGrammarForDisallowedTrailingComma(list: NodeArray<Node>, diag = Diagnostics.Trailing_comma_not_allowed): boolean { |
There was a problem hiding this comment.
Where is this optional parameter used?
There was a problem hiding this comment.
See the four new calls added in this PR.
|
@Andy-MS please add a blurb about this change in https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes |
DanielRosenwasser
modified the milestones:
TypeScript 2.8,
TypeScript 2.8.1,
TypeScript 2.9
|
@mhegazy Good to merge now that 2.8 is released? |
|
👍 |
|
@ajafff I run into this issue with
After some search, I did not find the option you mentioned, would be appreciate if you could post it here, thanks! |
Since TS v2.9 trailing comma is not allowed after rest parameters or bindings. See: microsoft/TypeScript#22262 This PR is just aligning with new TS requirement to avoid conflicts.
6 participants
Fixes babel/babel#7460 (comment)
This will likely be annoying for many users, so we should make sure this definitely isn't going to be allowed by any future ES spec.
(MDN ref: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Trailing_commas#Trailing_commas_in_functions)