Skip to content

Commit ac147b1

Browse files
committed
Merge pull request microsoft#7442 from Victorystick/fix-property-shorthand-emit
Fix shorthand properties for non-es6 module formats
2 parents b29f460 + 2e23010 commit ac147b1

10 files changed

+129
-9
lines changed

src/compiler/emitter.ts

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -296,21 +296,21 @@ namespace ts {
296296
* If loop contains block scoped binding captured in some function then loop body is converted to a function.
297297
* Lexical bindings declared in loop initializer will be passed into the loop body function as parameters,
298298
* however if this binding is modified inside the body - this new value should be propagated back to the original binding.
299-
* This is done by declaring new variable (out parameter holder) outside of the loop for every binding that is reassigned inside the body.
299+
* This is done by declaring new variable (out parameter holder) outside of the loop for every binding that is reassigned inside the body.
300300
* On every iteration this variable is initialized with value of corresponding binding.
301301
* At every point where control flow leaves the loop either explicitly (break/continue) or implicitly (at the end of loop body)
302302
* we copy the value inside the loop to the out parameter holder.
303-
*
303+
*
304304
* for (let x;;) {
305305
* let a = 1;
306306
* let b = () => a;
307307
* x++
308308
* if (...) break;
309309
* ...
310310
* }
311-
*
311+
*
312312
* will be converted to
313-
*
313+
*
314314
* var out_x;
315315
* var loop = function(x) {
316316
* var a = 1;
@@ -326,7 +326,7 @@ namespace ts {
326326
* x = out_x;
327327
* if (state === "break") break;
328328
* }
329-
*
329+
*
330330
* NOTE: values to out parameters are not copies if loop is abrupted with 'return' - in this case this will end the entire enclosing function
331331
* so nobody can observe this new value.
332332
*/
@@ -2138,6 +2138,12 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
21382138
return container && container.kind !== SyntaxKind.SourceFile;
21392139
}
21402140

2141+
// Return true if identifier resolves to an imported identifier
2142+
function isImportedReference(node: Identifier) {
2143+
const declaration = resolver.getReferencedImportDeclaration(node);
2144+
return declaration && (declaration.kind === SyntaxKind.ImportClause || declaration.kind === SyntaxKind.ImportSpecifier);
2145+
}
2146+
21412147
function emitShorthandPropertyAssignment(node: ShorthandPropertyAssignment) {
21422148
// The name property of a short-hand property assignment is considered an expression position, so here
21432149
// we manually emit the identifier to avoid rewriting.
@@ -2151,7 +2157,18 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
21512157
// let obj = { y };
21522158
// }
21532159
// Here we need to emit obj = { y : m.y } regardless of the output target.
2154-
if (modulekind !== ModuleKind.ES6 || isNamespaceExportReference(node.name)) {
2160+
// The same rules apply for imported identifiers when targeting module formats with indirect access to
2161+
// the imported identifiers. For example, when targeting CommonJS:
2162+
//
2163+
// import {foo} from './foo';
2164+
// export const baz = { foo };
2165+
//
2166+
// Must be transformed into:
2167+
//
2168+
// const foo_1 = require('./foo');
2169+
// exports.baz = { foo: foo_1.foo };
2170+
//
2171+
if (languageVersion < ScriptTarget.ES6 || (modulekind !== ModuleKind.ES6 && isImportedReference(node.name)) || isNamespaceExportReference(node.name) ) {
21552172
// Emit identifier as an identifier
21562173
write(": ");
21572174
emit(node.name);
@@ -3073,7 +3090,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
30733090
}
30743091

30753092
writeLine();
3076-
// end of loop body -> copy out parameter
3093+
// end of loop body -> copy out parameter
30773094
copyLoopOutParameters(convertedLoopState, CopyDirection.ToOutParameter, /*emitAsStatements*/true);
30783095

30793096
decreaseIndent();
@@ -3572,7 +3589,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
35723589
}
35733590
else {
35743591
convertedLoopState.nonLocalJumps |= Jump.Continue;
3575-
// note: return value is emitted only to simplify debugging, call to converted loop body does not do any dispatching on it.
3592+
// note: return value is emitted only to simplify debugging, call to converted loop body does not do any dispatching on it.
35763593
write(`"continue";`);
35773594
}
35783595
}
@@ -7267,7 +7284,7 @@ const _super = (function (geti, seti) {
72677284
}
72687285

72697286
// text should be quoted string
7270-
// for deduplication purposes in key remove leading and trailing quotes so 'a' and "a" will be considered the same
7287+
// for deduplication purposes in key remove leading and trailing quotes so 'a' and "a" will be considered the same
72717288
const key = text.substr(1, text.length - 2);
72727289

72737290
if (hasProperty(groupIndices, key)) {
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
error TS1204: Cannot compile modules into 'es2015' when targeting 'ES5' or lower.
2+
tests/cases/compiler/test.ts(2,19): error TS2307: Cannot find module './foo'.
3+
4+
5+
!!! error TS1204: Cannot compile modules into 'es2015' when targeting 'ES5' or lower.
6+
==== tests/cases/compiler/test.ts (1 errors) ====
7+
8+
import {foo} from './foo';
9+
~~~~~~~
10+
!!! error TS2307: Cannot find module './foo'.
11+
const baz = 42;
12+
const bar = { foo, baz };
13+
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
//// [test.ts]
2+
3+
import {foo} from './foo';
4+
const baz = 42;
5+
const bar = { foo, baz };
6+
7+
8+
//// [test.js]
9+
import { foo } from './foo';
10+
var baz = 42;
11+
var bar = { foo: foo, baz: baz };
12+
13+
14+
//// [test.d.ts]
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
tests/cases/compiler/test.ts(2,19): error TS2307: Cannot find module './foo'.
2+
3+
4+
==== tests/cases/compiler/test.ts (1 errors) ====
5+
6+
import {foo} from './foo';
7+
~~~~~~~
8+
!!! error TS2307: Cannot find module './foo'.
9+
const baz = 42;
10+
const bar = { foo, baz };
11+
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
//// [test.ts]
2+
3+
import {foo} from './foo';
4+
const baz = 42;
5+
const bar = { foo, baz };
6+
7+
8+
//// [test.js]
9+
define(["require", "exports", './foo'], function (require, exports, foo_1) {
10+
"use strict";
11+
const baz = 42;
12+
const bar = { foo: foo_1.foo, baz };
13+
});
14+
15+
16+
//// [test.d.ts]
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
tests/cases/compiler/test.ts(2,19): error TS2307: Cannot find module './foo'.
2+
3+
4+
==== tests/cases/compiler/test.ts (1 errors) ====
5+
6+
import {foo} from './foo';
7+
~~~~~~~
8+
!!! error TS2307: Cannot find module './foo'.
9+
const baz = 42;
10+
const bar = { foo, baz };
11+
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
//// [test.ts]
2+
3+
import {foo} from './foo';
4+
const baz = 42;
5+
const bar = { foo, baz };
6+
7+
8+
//// [test.js]
9+
import { foo } from './foo';
10+
const baz = 42;
11+
const bar = { foo, baz };
12+
13+
14+
//// [test.d.ts]
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// @target: ES5
2+
// @module: ES6
3+
// @declaration: true
4+
5+
// @filename: test.ts
6+
import {foo} from './foo';
7+
const baz = 42;
8+
const bar = { foo, baz };
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// @target: ES6
2+
// @module: amd
3+
// @declaration: true
4+
5+
// @filename: test.ts
6+
import {foo} from './foo';
7+
const baz = 42;
8+
const bar = { foo, baz };
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// @target: ES6
2+
// @module: ES6
3+
// @declaration: true
4+
5+
// @filename: test.ts
6+
import {foo} from './foo';
7+
const baz = 42;
8+
const bar = { foo, baz };

0 commit comments

Comments
 (0)