Conversation
| let typeAssertionExpression = <TypeAssertion>node; | ||
| diagnostics.push(createDiagnosticForNode(typeAssertionExpression.type, Diagnostics.type_assertion_expressions_can_only_be_used_in_a_ts_file)); | ||
| return true; | ||
| case SyntaxKind.Decorator: |
There was a problem hiding this comment.
since this is a non-standard feature, we should ask you to set a flag.. i.e. error if the flag is not set, and ask the user to specify the flag.
There was a problem hiding this comment.
Hmmmm. I'm conflicted on this one, as it seems maybe a little user hostile. (Kind of like we used to require a 'module' setting for you to write a module). Its not like they are going to write valid decorator syntax by accident, they opt in to decorators by virtue of writing one, and there is only one setting for the config option (true), but we'd go make them add a config file and the option anyway, (even though for many Salsa users they are just editing code, not compiling).
On the other hand, I get that the feature is "experimental" and we have precedent. Let's chat tomorrow, and I'll try and prepare the change anyway in the meantime. ( @egamma Any strong thoughts?).
There was a problem hiding this comment.
i am just worried of breaking these users later with no warning. the switch is a implicit agreement that you may be broken. not sure if this changes your feelings when you really are broken though.
There was a problem hiding this comment.
My only strong feeling is that showing an error is too much, showing a warning that this is an experimental feature would be more appropriate. But then the warning has to tell me how to get rid of it.
|
I've updated this to give the same warning in JavaScript that it gives in TypeScript, namely to set I can port this to |
| "code": 1218 | ||
| }, | ||
| "Experimental support for decorators is a feature that is subject to change in a future release. Specify '--experimentalDecorators' to remove this warning.": { | ||
| "Experimental support for decorators is a feature that is subject to change in a future release. Set 'experimentalDecorators' to remove this warning.": { |
There was a problem hiding this comment.
"the 'experimentalDecorators' option"
|
Looks good to me. Just need to get the tests working (see #7069). |
|
sorry for the delay. 👍 |
|
👍 |
Allow decorators in JavaScript files
6 participants
Fixes #6872