Skip to content

Allow decorators in JavaScript files #6881

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Feb 16, 2016
Merged

Allow decorators in JavaScript files #6881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f601e6d
Allow decorators in JavaScript files
billti Feb 3, 2016
ed7abcc
Set experimentalDecorators warning for JavaScript
billti Feb 13, 2016
fb474d1
Updated tests
billti Feb 13, 2016
a4d7883
Merge branch 'master' into issue6872
billti Feb 13, 2016
fe60490
Fixed wording and updated tests
billti Feb 13, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12740,7 +12740,7 @@ namespace ts {
}

if (!compilerOptions.experimentalDecorators) {
error(node, Diagnostics.Experimental_support_for_decorators_is_a_feature_that_is_subject_to_change_in_a_future_release_Specify_experimentalDecorators_to_remove_this_warning);
error(node, Diagnostics.Experimental_support_for_decorators_is_a_feature_that_is_subject_to_change_in_a_future_release_Set_the_experimentalDecorators_option_to_remove_this_warning);
}

if (compilerOptions.emitDecoratorMetadata) {
Expand Down
7 changes: 1 addition & 6 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@
"category": "Error",
"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 the 'experimentalDecorators' option to remove this warning.": {
"category": "Error",
"code": 1219
},
Expand Down Expand Up @@ -2729,11 +2729,6 @@
"category": "Error",
"code": 8016
},
"'decorators' can only be used in a .ts file.": {
"category": "Error",
"code": 8017
},

"Only identifiers/qualified-names with optional type arguments are currently supported in a class 'extends' clauses.": {
"category": "Error",
"code": 9002
Expand Down
4 changes: 3 additions & 1 deletion src/compiler/program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1169,7 +1169,9 @@ namespace ts {
diagnostics.push(createDiagnosticForNode(typeAssertionExpression.type, Diagnostics.type_assertion_expressions_can_only_be_used_in_a_ts_file));
return true;
case SyntaxKind.Decorator:
Copy link
Contributor

@mhegazy mhegazy Feb 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@billti billti Feb 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?).

Copy link
Contributor

@mhegazy mhegazy Feb 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@egamma egamma Feb 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

diagnostics.push(createDiagnosticForNode(node, Diagnostics.decorators_can_only_be_used_in_a_ts_file));
if (!options.experimentalDecorators) {
diagnostics.push(createDiagnosticForNode(node, Diagnostics.Experimental_support_for_decorators_is_a_feature_that_is_subject_to_change_in_a_future_release_Set_the_experimentalDecorators_option_to_remove_this_warning));
}
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/generatorTypeCheck39.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck39.ts(5,16): error TS1163: A 'yield' expression is only allowed in a generator body.
tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck39.ts(6,11): error TS1219: Experimental support for decorators is a feature that is subject to change in a future release. Specify '--experimentalDecorators' to remove this warning.
tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck39.ts(6,11): error TS1219: Experimental support for decorators is a feature that is subject to change in a future release. Set the 'experimentalDecorators' option to remove this warning.
tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck39.ts(7,13): error TS1163: A 'yield' expression is only allowed in a generator body.


Expand All @@ -13,7 +13,7 @@ tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck39.ts(7,13): erro
!!! error TS1163: A 'yield' expression is only allowed in a generator body.
class C {
~
Copy link
Member

@DanielRosenwasser DanielRosenwasser Feb 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's kind of weird that we error on the node instead of the decorator itself. We should consider that in a separate change.

!!! error TS1219: Experimental support for decorators is a feature that is subject to change in a future release. Specify '--experimentalDecorators' to remove this warning.
!!! error TS1219: Experimental support for decorators is a feature that is subject to change in a future release. Set the 'experimentalDecorators' option to remove this warning.
x = yield 0;
~~~~~
!!! error TS1163: A 'yield' expression is only allowed in a generator body.
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/generatorTypeCheck59.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck59.ts(3,11): error TS1163: A 'yield' expression is only allowed in a generator body.
tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck59.ts(4,9): error TS1219: Experimental support for decorators is a feature that is subject to change in a future release. Specify '--experimentalDecorators' to remove this warning.
tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck59.ts(4,9): error TS1219: Experimental support for decorators is a feature that is subject to change in a future release. Set the 'experimentalDecorators' option to remove this warning.


==== tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck59.ts (2 errors) ====
Expand All @@ -10,6 +10,6 @@ tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck59.ts(4,9): error
!!! error TS1163: A 'yield' expression is only allowed in a generator body.
m() { }
~
!!! error TS1219: Experimental support for decorators is a feature that is subject to change in a future release. Specify '--experimentalDecorators' to remove this warning.
!!! error TS1219: Experimental support for decorators is a feature that is subject to change in a future release. Set the 'experimentalDecorators' option to remove this warning.
};
}
4 changes: 2 additions & 2 deletions tests/baselines/reference/generatorTypeCheck61.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck61.ts(2,7): error TS1163: A 'yield' expression is only allowed in a generator body.
tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck61.ts(3,11): error TS1219: Experimental support for decorators is a feature that is subject to change in a future release. Specify '--experimentalDecorators' to remove this warning.
tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck61.ts(3,11): error TS1219: Experimental support for decorators is a feature that is subject to change in a future release. Set the 'experimentalDecorators' option to remove this warning.


==== tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck61.ts (2 errors) ====
Expand All @@ -9,5 +9,5 @@ tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck61.ts(3,11): erro
!!! error TS1163: A 'yield' expression is only allowed in a generator body.
class C {};
~
!!! error TS1219: Experimental support for decorators is a feature that is subject to change in a future release. Specify '--experimentalDecorators' to remove this warning.
!!! error TS1219: Experimental support for decorators is a feature that is subject to change in a future release. Set the 'experimentalDecorators' option to remove this warning.
}
9 changes: 0 additions & 9 deletions tests/baselines/reference/jsFileCompilationDecoratorSyntax.errors.txt
View file
Open in desktop

This file was deleted.

4 changes: 4 additions & 0 deletions tests/baselines/reference/jsFileCompilationDecoratorSyntax.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
=== tests/cases/compiler/a.js ===
@internal class C { }
>C : Symbol(C, Decl(a.js, 0, 0))

5 changes: 5 additions & 0 deletions tests/baselines/reference/jsFileCompilationDecoratorSyntax.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
=== tests/cases/compiler/a.js ===
@internal class C { }
>internal : any
>C : C

4 changes: 3 additions & 1 deletion tests/cases/compiler/jsFileCompilationDecoratorSyntax.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// @allowJs: true
// @noEmit: true
// @experimentalDecorators: true
// @filename: a.js
@internal class C { }
@internal class C { }
11 changes: 2 additions & 9 deletions tests/cases/fourslash/getJavaScriptSemanticDiagnostics21.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
/// <reference path="fourslash.ts" />

// @allowJs: true
// @experimentalDecorators: true
// @Filename: a.js
//// @internal class C {}

verify.getSemanticDiagnostics(`[
{
"message": "'decorators' can only be used in a .ts file.",
"start": 0,
"length": 9,
"category": "error",
"code": 8017
}
]`);
verify.getSemanticDiagnostics(`[]`);
Copy link
Member

@DanielRosenwasser DanielRosenwasser Feb 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider just verify.numberOfErrorsInCurrentFile(0);