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.

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

Support modules when targeting ES6 and an ES6 ModuleKind #4811

Merged
merged 8 commits into from
Sep 24, 2015
Merged

Support modules when targeting ES6 and an ES6 ModuleKind #4811

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
48ba708
Add es6 module kind, stop using script version to infer it
weswigham Sep 15, 2015
c71a0ac
Pair of new tests for the new flag
weswigham Sep 15, 2015
60a120f
remove es6 as option from command line
weswigham Sep 16, 2015
75a5c22
Fix import/export assignment checks to look at module kind rather tha…
weswigham Sep 16, 2015
f2b901a
reverts 60a120f, reexposes es6 option
weswigham Sep 17, 2015
8d6cb5f
Feedback from PR
weswigham Sep 17, 2015
04b0aa2
s/var/let
weswigham Sep 18, 2015
8ff551c
merge with master
weswigham Sep 22, 2015
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
13 changes: 7 additions & 6 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ namespace ts {

let compilerOptions = host.getCompilerOptions();
let languageVersion = compilerOptions.target || ScriptTarget.ES3;
var modulekind = compilerOptions.module ? compilerOptions.module : languageVersion === ScriptTarget.ES6 ? ModuleKind.ES6 : ModuleKind.None;
Copy link
Member

Choose a reason for hiding this comment

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

s/var/let

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps it would be better to remove ModuleKind.ES6, as it could be confusing to API consumers, and instead use this here:

let moduleKind = compilerOptions.module || ModuleKind.None;
let nativeModules = languageVersion >= ScriptTarget.ES6 && moduleKind === ModuleKind.None;

Then use the nativeModules boolean variable when checking below, or merely inline the expression used to compute it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps it would be better to remove ModuleKind.ES6, as it could be confusing to API consumers, and instead use this here

Please no. Not having ES6 as a module kind is all kinds of not forward compatible and causes odd checks which depend on there not being module kinds alongside specific language versions. Going forward, when we implement #4813 those forms of exports will not be ES6 compatible module emit, but can be compiled to ES6 compatible module emit. In effect, when we add an ES7 target and ES7 compatible module emit feature, we're going to need an ES6 module kind (and, to be consistent, hopefully an ES7 module kind!). Additionally, not having an ES6 module kind makes specifying that you want to emit an ES6 module style bundle for #4754 awkward. (Do I target ES6 but not specify module output type at all? That's oddly inconsistent.)

--module is one of our few variable feature flags at present, it's time we start treating it as such. ES6 is a kind of module emit. We should treat it as such. The module format is part of the ES6 spec, true, but nothing inherently ties the feature to the ES6 target only. Right now it seems nonsensical to ask for ES6 module syntax with a target below ES6 (since we know of 0 runtimes which support ES6 modules and yet support no other ES6 features), but we certainly could emit it.

IMO, having an ES6 module kind simplifies our API. When a module kind is set, it now controls the emit for the module format. Full stop.


let emitResolver = createResolver();

Expand Down Expand Up @@ -13379,9 +13380,9 @@ namespace ts {
}
}
else {
if (languageVersion >= ScriptTarget.ES6 && !isInAmbientContext(node)) {
if (modulekind === ModuleKind.ES6 && !isInAmbientContext(node)) {
// Import equals declaration is deprecated in es6 or above
grammarErrorOnNode(node, Diagnostics.Import_assignment_cannot_be_used_when_targeting_ECMAScript_6_or_higher_Consider_using_import_Asterisk_as_ns_from_mod_import_a_from_mod_or_import_d_from_mod_instead);
grammarErrorOnNode(node, Diagnostics.Import_assignment_cannot_be_used_when_targeting_ECMAScript_6_modules_Consider_using_import_Asterisk_as_ns_from_mod_import_a_from_mod_import_d_from_mod_or_another_module_format_instead);
}
}
}
Expand Down Expand Up @@ -13456,11 +13457,11 @@ namespace ts {
checkExternalModuleExports(<SourceFile | ModuleDeclaration>container);

if (node.isExportEquals && !isInAmbientContext(node)) {
if (languageVersion >= ScriptTarget.ES6) {
// export assignment is deprecated in es6 or above
grammarErrorOnNode(node, Diagnostics.Export_assignment_cannot_be_used_when_targeting_ECMAScript_6_or_higher_Consider_using_export_default_instead);
if (modulekind === ModuleKind.ES6) {
// export assignment is not supported in es6 modules
grammarErrorOnNode(node, Diagnostics.Export_assignment_cannot_be_used_when_targeting_ECMAScript_6_modules_Consider_using_export_default_or_another_module_format_instead);
}
else if (compilerOptions.module === ModuleKind.System) {
else if (modulekind === ModuleKind.System) {
// system modules does not support export assignment
grammarErrorOnNode(node, Diagnostics.Export_assignment_is_not_supported_when_module_flag_is_system);
}
Expand Down
5 changes: 3 additions & 2 deletions src/compiler/commandLineParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,11 @@ namespace ts {
"amd": ModuleKind.AMD,
"system": ModuleKind.System,
"umd": ModuleKind.UMD,
"es6": ModuleKind.ES6,
Copy link
Contributor

Choose a reason for hiding this comment

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

you do not need that. just leave target as es6 and allow for additional --module flag. today we make it an error to use --target es6 with --module; relaxing this should be sufficient.

Copy link
Member Author

Choose a reason for hiding this comment

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

Internally we checked for language version like it was a module kind in a number of places. Is it just a matter of not wanting to make it able to be specified explicitly?

Copy link
Contributor

Choose a reason for hiding this comment

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

we do not support target: es5 with module: es6, so why allow it and add additional burden on users.
--target es6 with no --module is implied to be es6, if another --module tareget is specified, emit should use that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright, so I'm just removing it as an option for the --module command line flag.

},
description: Diagnostics.Specify_module_code_generation_Colon_commonjs_amd_system_or_umd,
description: Diagnostics.Specify_module_code_generation_Colon_commonjs_amd_system_umd_or_es6,
paramType: Diagnostics.KIND,
error: Diagnostics.Argument_for_module_option_must_be_commonjs_amd_system_or_umd
error: Diagnostics.Argument_for_module_option_must_be_commonjs_amd_system_umd_or_es6
},
{
name: "newLine",
Expand Down
10 changes: 5 additions & 5 deletions src/compiler/diagnosticInformationMap.generated.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,9 @@ namespace ts {
An_extended_Unicode_escape_value_must_be_between_0x0_and_0x10FFFF_inclusive: { code: 1198, category: DiagnosticCategory.Error, key: "An extended Unicode escape value must be between 0x0 and 0x10FFFF inclusive." },
Unterminated_Unicode_escape_sequence: { code: 1199, category: DiagnosticCategory.Error, key: "Unterminated Unicode escape sequence." },
Line_terminator_not_permitted_before_arrow: { code: 1200, category: DiagnosticCategory.Error, key: "Line terminator not permitted before arrow." },
Import_assignment_cannot_be_used_when_targeting_ECMAScript_6_or_higher_Consider_using_import_Asterisk_as_ns_from_mod_import_a_from_mod_or_import_d_from_mod_instead: { code: 1202, category: DiagnosticCategory.Error, key: "Import assignment cannot be used when targeting ECMAScript 6 or higher. Consider using 'import * as ns from \"mod\"', 'import {a} from \"mod\"' or 'import d from \"mod\"' instead." },
Export_assignment_cannot_be_used_when_targeting_ECMAScript_6_or_higher_Consider_using_export_default_instead: { code: 1203, category: DiagnosticCategory.Error, key: "Export assignment cannot be used when targeting ECMAScript 6 or higher. Consider using 'export default' instead." },
Cannot_compile_modules_into_commonjs_amd_system_or_umd_when_targeting_ES6_or_higher: { code: 1204, category: DiagnosticCategory.Error, key: "Cannot compile modules into 'commonjs', 'amd', 'system' or 'umd' when targeting 'ES6' or higher." },
Import_assignment_cannot_be_used_when_targeting_ECMAScript_6_modules_Consider_using_import_Asterisk_as_ns_from_mod_import_a_from_mod_import_d_from_mod_or_another_module_format_instead: { code: 1202, category: DiagnosticCategory.Error, key: "Import assignment cannot be used when targeting ECMAScript 6 modules. Consider using 'import * as ns from \"mod\"', 'import {a} from \"mod\"', 'import d from \"mod\"', or another module format instead." },
Export_assignment_cannot_be_used_when_targeting_ECMAScript_6_modules_Consider_using_export_default_or_another_module_format_instead: { code: 1203, category: DiagnosticCategory.Error, key: "Export assignment cannot be used when targeting ECMAScript 6 modules. Consider using 'export default' or another module format instead." },
Cannot_compile_modules_into_es6_when_targeting_ES5_or_lower: { code: 1204, category: DiagnosticCategory.Error, key: "Cannot compile modules into 'es6' when targeting 'ES5' or lower." },
Decorators_are_only_available_when_targeting_ECMAScript_5_and_higher: { code: 1205, category: DiagnosticCategory.Error, key: "Decorators are only available when targeting ECMAScript 5 and higher." },
Decorators_are_not_valid_here: { code: 1206, category: DiagnosticCategory.Error, key: "Decorators are not valid here." },
Decorators_cannot_be_applied_to_multiple_get_Slashset_accessors_of_the_same_name: { code: 1207, category: DiagnosticCategory.Error, key: "Decorators cannot be applied to multiple get/set accessors of the same name." },
Expand Down Expand Up @@ -528,7 +528,7 @@ namespace ts {
Do_not_emit_comments_to_output: { code: 6009, category: DiagnosticCategory.Message, key: "Do not emit comments to output." },
Do_not_emit_outputs: { code: 6010, category: DiagnosticCategory.Message, key: "Do not emit outputs." },
Specify_ECMAScript_target_version_Colon_ES3_default_ES5_or_ES6_experimental: { code: 6015, category: DiagnosticCategory.Message, key: "Specify ECMAScript target version: 'ES3' (default), 'ES5', or 'ES6' (experimental)" },
Specify_module_code_generation_Colon_commonjs_amd_system_or_umd: { code: 6016, category: DiagnosticCategory.Message, key: "Specify module code generation: 'commonjs', 'amd', 'system' or 'umd'" },
Specify_module_code_generation_Colon_commonjs_amd_system_umd_or_es6: { code: 6016, category: DiagnosticCategory.Message, key: "Specify module code generation: 'commonjs', 'amd', 'system', 'umd' or 'es6'" },
Print_this_message: { code: 6017, category: DiagnosticCategory.Message, key: "Print this message." },
Print_the_compiler_s_version: { code: 6019, category: DiagnosticCategory.Message, key: "Print the compiler's version." },
Compile_the_project_in_the_given_directory: { code: 6020, category: DiagnosticCategory.Message, key: "Compile the project in the given directory." },
Expand All @@ -549,7 +549,7 @@ namespace ts {
Generates_corresponding_map_file: { code: 6043, category: DiagnosticCategory.Message, key: "Generates corresponding '.map' file." },
Compiler_option_0_expects_an_argument: { code: 6044, category: DiagnosticCategory.Error, key: "Compiler option '{0}' expects an argument." },
Unterminated_quoted_string_in_response_file_0: { code: 6045, category: DiagnosticCategory.Error, key: "Unterminated quoted string in response file '{0}'." },
Argument_for_module_option_must_be_commonjs_amd_system_or_umd: { code: 6046, category: DiagnosticCategory.Error, key: "Argument for '--module' option must be 'commonjs', 'amd', 'system' or 'umd'." },
Argument_for_module_option_must_be_commonjs_amd_system_umd_or_es6: { code: 6046, category: DiagnosticCategory.Error, key: "Argument for '--module' option must be 'commonjs', 'amd', 'system', 'umd', or 'es6'." },
Argument_for_target_option_must_be_ES3_ES5_or_ES6: { code: 6047, category: DiagnosticCategory.Error, key: "Argument for '--target' option must be 'ES3', 'ES5', or 'ES6'." },
Locale_must_be_of_the_form_language_or_language_territory_For_example_0_or_1: { code: 6048, category: DiagnosticCategory.Error, key: "Locale must be of the form <language> or <language>-<territory>. For example '{0}' or '{1}'." },
Unsupported_locale_0: { code: 6049, category: DiagnosticCategory.Error, key: "Unsupported locale '{0}'." },
Expand Down
10 changes: 5 additions & 5 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,15 +619,15 @@
"category": "Error",
"code": 1200
},
"Import assignment cannot be used when targeting ECMAScript 6 or higher. Consider using 'import * as ns from \"mod\"', 'import {a} from \"mod\"' or 'import d from \"mod\"' instead.": {
"Import assignment cannot be used when targeting ECMAScript 6 modules. Consider using 'import * as ns from \"mod\"', 'import {a} from \"mod\"', 'import d from \"mod\"', or another module format instead.": {
"category": "Error",
"code": 1202
},
"Export assignment cannot be used when targeting ECMAScript 6 or higher. Consider using 'export default' instead.": {
"Export assignment cannot be used when targeting ECMAScript 6 modules. Consider using 'export default' or another module format instead.": {
"category": "Error",
"code": 1203
},
"Cannot compile modules into 'commonjs', 'amd', 'system' or 'umd' when targeting 'ES6' or higher.": {
"Cannot compile modules into 'es6' when targeting 'ES5' or lower.": {
"category": "Error",
"code": 1204
},
Expand Down Expand Up @@ -2102,7 +2102,7 @@
"category": "Message",
"code": 6015
},
"Specify module code generation: 'commonjs', 'amd', 'system' or 'umd'": {
"Specify module code generation: 'commonjs', 'amd', 'system', 'umd' or 'es6'": {
"category": "Message",
"code": 6016
},
Expand Down Expand Up @@ -2186,7 +2186,7 @@
"category": "Error",
"code": 6045
},
"Argument for '--module' option must be 'commonjs', 'amd', 'system' or 'umd'.": {
"Argument for '--module' option must be 'commonjs', 'amd', 'system', 'umd', or 'es6'.": {
"category": "Error",
"code": 6046
},
Expand Down
Loading