From d742ca50f4b6199a6c14ffc78bdef261f43a73aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Segersva=CC=88rd?= Date: Wed, 9 Mar 2016 09:41:14 +0100 Subject: [PATCH 1/3] Fix shorthand properties for non-es6 module formats --- src/compiler/emitter.ts | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index aaee1e1ef99..67f9765d821 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -296,11 +296,11 @@ namespace ts { * If loop contains block scoped binding captured in some function then loop body is converted to a function. * Lexical bindings declared in loop initializer will be passed into the loop body function as parameters, * however if this binding is modified inside the body - this new value should be propagated back to the original binding. - * This is done by declaring new variable (out parameter holder) outside of the loop for every binding that is reassigned inside the body. + * This is done by declaring new variable (out parameter holder) outside of the loop for every binding that is reassigned inside the body. * On every iteration this variable is initialized with value of corresponding binding. * At every point where control flow leaves the loop either explicitly (break/continue) or implicitly (at the end of loop body) * we copy the value inside the loop to the out parameter holder. - * + * * for (let x;;) { * let a = 1; * let b = () => a; @@ -308,9 +308,9 @@ namespace ts { * if (...) break; * ... * } - * + * * will be converted to - * + * * var out_x; * var loop = function(x) { * var a = 1; @@ -326,7 +326,7 @@ namespace ts { * x = out_x; * if (state === "break") break; * } - * + * * NOTE: values to out parameters are not copies if loop is abrupted with 'return' - in this case this will end the entire enclosing function * so nobody can observe this new value. */ @@ -379,6 +379,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge const compilerOptions = host.getCompilerOptions(); const languageVersion = getEmitScriptTarget(compilerOptions); const modulekind = getEmitModuleKind(compilerOptions); + const hasIndirectAccessToImportedIdentifiers = modulekind !== ModuleKind.ES6 && modulekind !== ModuleKind.System; const sourceMapDataList: SourceMapData[] = compilerOptions.sourceMap || compilerOptions.inlineSourceMap ? [] : undefined; const emitterDiagnostics = createDiagnosticCollection(); let emitSkipped = false; @@ -1575,7 +1576,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge if (container) { if (container.kind === SyntaxKind.SourceFile) { // Identifier references module export - if (modulekind !== ModuleKind.ES6 && modulekind !== ModuleKind.System) { + if (hasIndirectAccessToImportedIdentifiers) { write("exports."); } } @@ -2138,6 +2139,12 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge return container && container.kind !== SyntaxKind.SourceFile; } + // Return true if identifier resolves to an imported identifier + function isImportedReference(node: Identifier) { + const declaration = resolver.getReferencedImportDeclaration(node); + return declaration && (declaration.kind === SyntaxKind.ImportClause || declaration.kind === SyntaxKind.ImportSpecifier); + } + function emitShorthandPropertyAssignment(node: ShorthandPropertyAssignment) { // The name property of a short-hand property assignment is considered an expression position, so here // we manually emit the identifier to avoid rewriting. @@ -2151,7 +2158,18 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge // let obj = { y }; // } // Here we need to emit obj = { y : m.y } regardless of the output target. - if (modulekind !== ModuleKind.ES6 || isNamespaceExportReference(node.name)) { + // The same rules apply for imported identifiers when targeting module formats with indirect access to + // the imported identifiers. For example, when targeting CommonJS: + // + // import {foo} from './foo'; + // export const baz = { foo }; + // + // Must be transformed into: + // + // const foo_1 = require('./foo'); + // exports.baz = { foo: foo_1.foo }; + // + if (languageVersion < ScriptTarget.ES6 || (hasIndirectAccessToImportedIdentifiers && isImportedReference(node.name)) || isNamespaceExportReference(node.name) ) { // Emit identifier as an identifier write(": "); emit(node.name); @@ -3073,7 +3091,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge } writeLine(); - // end of loop body -> copy out parameter + // end of loop body -> copy out parameter copyLoopOutParameters(convertedLoopState, CopyDirection.ToOutParameter, /*emitAsStatements*/true); decreaseIndent(); @@ -3572,7 +3590,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge } else { convertedLoopState.nonLocalJumps |= Jump.Continue; - // note: return value is emitted only to simplify debugging, call to converted loop body does not do any dispatching on it. + // note: return value is emitted only to simplify debugging, call to converted loop body does not do any dispatching on it. write(`"continue";`); } } @@ -7267,7 +7285,7 @@ const _super = (function (geti, seti) { } // text should be quoted string - // for deduplication purposes in key remove leading and trailing quotes so 'a' and "a" will be considered the same + // for deduplication purposes in key remove leading and trailing quotes so 'a' and "a" will be considered the same const key = text.substr(1, text.length - 2); if (hasProperty(groupIndices, key)) { From ccd5352eb88e1a5ebe6b7f5f8d00606b144bad8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Segersva=CC=88rd?= Date: Wed, 9 Mar 2016 12:13:27 +0100 Subject: [PATCH 2/3] System doesn't have direct identifier access in TS's generated code. --- src/compiler/emitter.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 67f9765d821..14f0e5bdf94 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -379,7 +379,6 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge const compilerOptions = host.getCompilerOptions(); const languageVersion = getEmitScriptTarget(compilerOptions); const modulekind = getEmitModuleKind(compilerOptions); - const hasIndirectAccessToImportedIdentifiers = modulekind !== ModuleKind.ES6 && modulekind !== ModuleKind.System; const sourceMapDataList: SourceMapData[] = compilerOptions.sourceMap || compilerOptions.inlineSourceMap ? [] : undefined; const emitterDiagnostics = createDiagnosticCollection(); let emitSkipped = false; @@ -1576,7 +1575,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge if (container) { if (container.kind === SyntaxKind.SourceFile) { // Identifier references module export - if (hasIndirectAccessToImportedIdentifiers) { + if (modulekind !== ModuleKind.ES6 && modulekind !== ModuleKind.System) { write("exports."); } } @@ -2169,7 +2168,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge // const foo_1 = require('./foo'); // exports.baz = { foo: foo_1.foo }; // - if (languageVersion < ScriptTarget.ES6 || (hasIndirectAccessToImportedIdentifiers && isImportedReference(node.name)) || isNamespaceExportReference(node.name) ) { + if (languageVersion < ScriptTarget.ES6 || (modulekind !== ModuleKind.ES6 && isImportedReference(node.name)) || isNamespaceExportReference(node.name) ) { // Emit identifier as an identifier write(": "); emit(node.name); From 2e23010437379e141af195999160162aa5b7e4c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Segersva=CC=88rd?= Date: Wed, 9 Mar 2016 18:43:21 +0100 Subject: [PATCH 3/3] Add three tests --- .../shorthand-property-es5-es6.errors.txt | 13 +++++++++++++ .../reference/shorthand-property-es5-es6.js | 14 ++++++++++++++ .../shorthand-property-es6-amd.errors.txt | 11 +++++++++++ .../reference/shorthand-property-es6-amd.js | 16 ++++++++++++++++ .../shorthand-property-es6-es6.errors.txt | 11 +++++++++++ .../reference/shorthand-property-es6-es6.js | 14 ++++++++++++++ .../cases/compiler/shorthand-property-es5-es6.ts | 8 ++++++++ .../cases/compiler/shorthand-property-es6-amd.ts | 8 ++++++++ .../cases/compiler/shorthand-property-es6-es6.ts | 8 ++++++++ 9 files changed, 103 insertions(+) create mode 100644 tests/baselines/reference/shorthand-property-es5-es6.errors.txt create mode 100644 tests/baselines/reference/shorthand-property-es5-es6.js create mode 100644 tests/baselines/reference/shorthand-property-es6-amd.errors.txt create mode 100644 tests/baselines/reference/shorthand-property-es6-amd.js create mode 100644 tests/baselines/reference/shorthand-property-es6-es6.errors.txt create mode 100644 tests/baselines/reference/shorthand-property-es6-es6.js create mode 100644 tests/cases/compiler/shorthand-property-es5-es6.ts create mode 100644 tests/cases/compiler/shorthand-property-es6-amd.ts create mode 100644 tests/cases/compiler/shorthand-property-es6-es6.ts diff --git a/tests/baselines/reference/shorthand-property-es5-es6.errors.txt b/tests/baselines/reference/shorthand-property-es5-es6.errors.txt new file mode 100644 index 00000000000..f491dfbaabe --- /dev/null +++ b/tests/baselines/reference/shorthand-property-es5-es6.errors.txt @@ -0,0 +1,13 @@ +error TS1204: Cannot compile modules into 'es2015' when targeting 'ES5' or lower. +tests/cases/compiler/test.ts(2,19): error TS2307: Cannot find module './foo'. + + +!!! error TS1204: Cannot compile modules into 'es2015' when targeting 'ES5' or lower. +==== tests/cases/compiler/test.ts (1 errors) ==== + + import {foo} from './foo'; + ~~~~~~~ +!!! error TS2307: Cannot find module './foo'. + const baz = 42; + const bar = { foo, baz }; + \ No newline at end of file diff --git a/tests/baselines/reference/shorthand-property-es5-es6.js b/tests/baselines/reference/shorthand-property-es5-es6.js new file mode 100644 index 00000000000..cbca92adb0c --- /dev/null +++ b/tests/baselines/reference/shorthand-property-es5-es6.js @@ -0,0 +1,14 @@ +//// [test.ts] + +import {foo} from './foo'; +const baz = 42; +const bar = { foo, baz }; + + +//// [test.js] +import { foo } from './foo'; +var baz = 42; +var bar = { foo: foo, baz: baz }; + + +//// [test.d.ts] diff --git a/tests/baselines/reference/shorthand-property-es6-amd.errors.txt b/tests/baselines/reference/shorthand-property-es6-amd.errors.txt new file mode 100644 index 00000000000..e54f73a4e61 --- /dev/null +++ b/tests/baselines/reference/shorthand-property-es6-amd.errors.txt @@ -0,0 +1,11 @@ +tests/cases/compiler/test.ts(2,19): error TS2307: Cannot find module './foo'. + + +==== tests/cases/compiler/test.ts (1 errors) ==== + + import {foo} from './foo'; + ~~~~~~~ +!!! error TS2307: Cannot find module './foo'. + const baz = 42; + const bar = { foo, baz }; + \ No newline at end of file diff --git a/tests/baselines/reference/shorthand-property-es6-amd.js b/tests/baselines/reference/shorthand-property-es6-amd.js new file mode 100644 index 00000000000..3cecc18b661 --- /dev/null +++ b/tests/baselines/reference/shorthand-property-es6-amd.js @@ -0,0 +1,16 @@ +//// [test.ts] + +import {foo} from './foo'; +const baz = 42; +const bar = { foo, baz }; + + +//// [test.js] +define(["require", "exports", './foo'], function (require, exports, foo_1) { + "use strict"; + const baz = 42; + const bar = { foo: foo_1.foo, baz }; +}); + + +//// [test.d.ts] diff --git a/tests/baselines/reference/shorthand-property-es6-es6.errors.txt b/tests/baselines/reference/shorthand-property-es6-es6.errors.txt new file mode 100644 index 00000000000..e54f73a4e61 --- /dev/null +++ b/tests/baselines/reference/shorthand-property-es6-es6.errors.txt @@ -0,0 +1,11 @@ +tests/cases/compiler/test.ts(2,19): error TS2307: Cannot find module './foo'. + + +==== tests/cases/compiler/test.ts (1 errors) ==== + + import {foo} from './foo'; + ~~~~~~~ +!!! error TS2307: Cannot find module './foo'. + const baz = 42; + const bar = { foo, baz }; + \ No newline at end of file diff --git a/tests/baselines/reference/shorthand-property-es6-es6.js b/tests/baselines/reference/shorthand-property-es6-es6.js new file mode 100644 index 00000000000..eff67c879cc --- /dev/null +++ b/tests/baselines/reference/shorthand-property-es6-es6.js @@ -0,0 +1,14 @@ +//// [test.ts] + +import {foo} from './foo'; +const baz = 42; +const bar = { foo, baz }; + + +//// [test.js] +import { foo } from './foo'; +const baz = 42; +const bar = { foo, baz }; + + +//// [test.d.ts] diff --git a/tests/cases/compiler/shorthand-property-es5-es6.ts b/tests/cases/compiler/shorthand-property-es5-es6.ts new file mode 100644 index 00000000000..8c8ae4368a0 --- /dev/null +++ b/tests/cases/compiler/shorthand-property-es5-es6.ts @@ -0,0 +1,8 @@ +// @target: ES5 +// @module: ES6 +// @declaration: true + +// @filename: test.ts +import {foo} from './foo'; +const baz = 42; +const bar = { foo, baz }; diff --git a/tests/cases/compiler/shorthand-property-es6-amd.ts b/tests/cases/compiler/shorthand-property-es6-amd.ts new file mode 100644 index 00000000000..0f2a62ad86c --- /dev/null +++ b/tests/cases/compiler/shorthand-property-es6-amd.ts @@ -0,0 +1,8 @@ +// @target: ES6 +// @module: amd +// @declaration: true + +// @filename: test.ts +import {foo} from './foo'; +const baz = 42; +const bar = { foo, baz }; diff --git a/tests/cases/compiler/shorthand-property-es6-es6.ts b/tests/cases/compiler/shorthand-property-es6-es6.ts new file mode 100644 index 00000000000..f904e4f8dcf --- /dev/null +++ b/tests/cases/compiler/shorthand-property-es6-es6.ts @@ -0,0 +1,8 @@ +// @target: ES6 +// @module: ES6 +// @declaration: true + +// @filename: test.ts +import {foo} from './foo'; +const baz = 42; +const bar = { foo, baz };