Skip to content

Commit dc607c2

Browse files
rbucktonrbuckton
committed
Fix 'this' capturing for dynamic import
1 parent 1db7623 commit dc607c2

10 files changed

Lines changed: 292 additions & 23 deletions

‎src/compiler/binder.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2699,6 +2699,12 @@ namespace ts {
26992699

27002700
if (expression.kind === SyntaxKind.ImportKeyword) {
27012701
transformFlags |= TransformFlags.ContainsDynamicImport;
2702+
2703+
// A dynamic 'import()' call that contains a lexical 'this' will
2704+
// require a captured 'this' when emitting down-level.
2705+
if (subtreeFlags & TransformFlags.ContainsLexicalThis) {
2706+
transformFlags |= TransformFlags.ContainsCapturedLexicalThis;
2707+
}
27022708
}
27032709

27042710
node.transformFlags = transformFlags | TransformFlags.HasComputedFlags;

‎src/compiler/transformers/module/module.ts‎

Lines changed: 66 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -561,46 +561,89 @@ namespace ts {
561561
// });
562562
const resolve = createUniqueName("resolve");
563563
const reject = createUniqueName("reject");
564-
return createNew(
565-
createIdentifier("Promise"),
566-
/*typeArguments*/ undefined,
567-
[createFunctionExpression(
564+
const parameters = [
565+
createParameter(/*decorator*/ undefined, /*modifiers*/ undefined, /*dotDotDotToken*/ undefined, /*name*/ resolve),
566+
createParameter(/*decorator*/ undefined, /*modifiers*/ undefined, /*dotDotDotToken*/ undefined, /*name*/ reject)
567+
];
568+
const body = createBlock([
569+
createStatement(
570+
createCall(
571+
createIdentifier("require"),
572+
/*typeArguments*/ undefined,
573+
[createArrayLiteral([firstOrUndefined(node.arguments) || createOmittedExpression()]), resolve, reject]
574+
)
575+
)
576+
]);
577+
578+
let func: FunctionExpression | ArrowFunction;
579+
if (languageVersion >= ScriptTarget.ES2015) {
580+
func = createArrowFunction(
581+
/*modifiers*/ undefined,
582+
/*typeParameters*/ undefined,
583+
parameters,
584+
/*type*/ undefined,
585+
/*equalsGreaterThanToken*/ undefined,
586+
body);
587+
}
588+
else {
589+
func = createFunctionExpression(
568590
/*modifiers*/ undefined,
569591
/*asteriskToken*/ undefined,
570592
/*name*/ undefined,
571593
/*typeParameters*/ undefined,
572-
[createParameter(/*decorator*/ undefined, /*modifiers*/ undefined, /*dotDotDotToken*/ undefined, /*name*/ resolve),
573-
createParameter(/*decorator*/ undefined, /*modifiers*/ undefined, /*dotDotDotToken*/ undefined, /*name*/ reject)],
594+
parameters,
574595
/*type*/ undefined,
575-
createBlock([createStatement(
576-
createCall(
577-
createIdentifier("require"),
578-
/*typeArguments*/ undefined,
579-
[createArrayLiteral([firstOrUndefined(node.arguments) || createOmittedExpression()]), resolve, reject]
580-
))])
581-
)]);
596+
body);
597+
598+
// if there is a lexical 'this' in the import call arguments, ensure we indicate
599+
// that this new function expression indicates it captures 'this' so that the
600+
// es2015 transformer will properly substitute 'this' with '_this'.
601+
if (node.transformFlags & TransformFlags.ContainsLexicalThis) {
602+
setEmitFlags(func, EmitFlags.CapturesThis);
603+
}
604+
}
605+
606+
return createNew(createIdentifier("Promise"), /*typeArguments*/ undefined, [func]);
582607
}
583608

584-
function transformImportCallExpressionCommonJS(node: ImportCall): Expression {
609+
function transformImportCallExpressionCommonJS(node: ImportCall): Expression {
585610
// import("./blah")
586611
// emit as
587612
// Promise.resolve().then(function () { return require(x); }) /*CommonJs Require*/
588613
// We have to wrap require in then callback so that require is done in asynchronously
589614
// if we simply do require in resolve callback in Promise constructor. We will execute the loading immediately
590-
return createCall(
591-
createPropertyAccess(
592-
createCall(createPropertyAccess(createIdentifier("Promise"), "resolve"), /*typeArguments*/ undefined, /*argumentsArray*/ []),
593-
"then"),
594-
/*typeArguments*/ undefined,
595-
[createFunctionExpression(
615+
const promiseResolveCall = createCall(createPropertyAccess(createIdentifier("Promise"), "resolve"), /*typeArguments*/ undefined, /*argumentsArray*/ []);
616+
const requireCall = createCall(createIdentifier("require"), /*typeArguments*/ undefined, node.arguments);
617+
618+
let func: FunctionExpression | ArrowFunction;
619+
if (languageVersion >= ScriptTarget.ES2015) {
620+
func = createArrowFunction(
621+
/*modifiers*/ undefined,
622+
/*typeParameters*/ undefined,
623+
/*parameters*/ [],
624+
/*type*/ undefined,
625+
/*equalsGreaterThanToken*/ undefined,
626+
requireCall);
627+
}
628+
else {
629+
func = createFunctionExpression(
596630
/*modifiers*/ undefined,
597631
/*asteriskToken*/ undefined,
598632
/*name*/ undefined,
599633
/*typeParameters*/ undefined,
600-
/*parameters*/ undefined,
634+
/*parameters*/ [],
601635
/*type*/ undefined,
602-
createBlock([createReturn(createCall(createIdentifier("require"), /*typeArguments*/ undefined, node.arguments))])
603-
)]);
636+
createBlock([createReturn(requireCall)]));
637+
638+
// if there is a lexical 'this' in the import call arguments, ensure we indicate
639+
// that this new function expression indicates it captures 'this' so that the
640+
// es2015 transformer will properly substitute 'this' with '_this'.
641+
if (node.transformFlags & TransformFlags.ContainsLexicalThis) {
642+
setEmitFlags(func, EmitFlags.CapturesThis);
643+
}
644+
}
645+
646+
return createCall(createPropertyAccess(promiseResolveCall, "then"), /*typeArguments*/ undefined, [func]);
604647
}
605648

606649
/**

‎tests/baselines/reference/dynamicImportWithNestedThis_es2015.js‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
//// [dynamicImportWithNestedThis_es2015.ts]
2+
// https://github.com/Microsoft/TypeScript/issues/17564
3+
class C {
4+
private _path = './other';
5+
6+
dynamic() {
7+
return import(this._path);
8+
}
9+
}
10+
11+
const c = new C();
12+
c.dynamic();
13+
14+
//// [dynamicImportWithNestedThis_es2015.js]
15+
(function (factory) {
16+
if (typeof module === "object" && typeof module.exports === "object") {
17+
var v = factory(require, exports);
18+
if (v !== undefined) module.exports = v;
19+
}
20+
else if (typeof define === "function" && define.amd) {
21+
define(["require", "exports"], factory);
22+
}
23+
})(function (require, exports) {
24+
"use strict";
25+
var __syncRequire = typeof module === "object" && typeof module.exports === "object";
26+
// https://github.com/Microsoft/TypeScript/issues/17564
27+
class C {
28+
constructor() {
29+
this._path = './other';
30+
}
31+
dynamic() {
32+
return __syncRequire ? Promise.resolve().then(() => require(this._path)) : new Promise((resolve_1, reject_1) => { require([this._path], resolve_1, reject_1); });
33+
}
34+
}
35+
const c = new C();
36+
c.dynamic();
37+
});

‎tests/baselines/reference/dynamicImportWithNestedThis_es2015.symbols‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
=== tests/cases/compiler/dynamicImportWithNestedThis_es2015.ts ===
2+
// https://github.com/Microsoft/TypeScript/issues/17564
3+
class C {
4+
>C : Symbol(C, Decl(dynamicImportWithNestedThis_es2015.ts, 0, 0))
5+
6+
private _path = './other';
7+
>_path : Symbol(C._path, Decl(dynamicImportWithNestedThis_es2015.ts, 1, 9))
8+
9+
dynamic() {
10+
>dynamic : Symbol(C.dynamic, Decl(dynamicImportWithNestedThis_es2015.ts, 2, 27))
11+
12+
return import(this._path);
13+
>this._path : Symbol(C._path, Decl(dynamicImportWithNestedThis_es2015.ts, 1, 9))
14+
>this : Symbol(C, Decl(dynamicImportWithNestedThis_es2015.ts, 0, 0))
15+
>_path : Symbol(C._path, Decl(dynamicImportWithNestedThis_es2015.ts, 1, 9))
16+
}
17+
}
18+
19+
const c = new C();
20+
>c : Symbol(c, Decl(dynamicImportWithNestedThis_es2015.ts, 9, 5))
21+
>C : Symbol(C, Decl(dynamicImportWithNestedThis_es2015.ts, 0, 0))
22+
23+
c.dynamic();
24+
>c.dynamic : Symbol(C.dynamic, Decl(dynamicImportWithNestedThis_es2015.ts, 2, 27))
25+
>c : Symbol(c, Decl(dynamicImportWithNestedThis_es2015.ts, 9, 5))
26+
>dynamic : Symbol(C.dynamic, Decl(dynamicImportWithNestedThis_es2015.ts, 2, 27))
27+

‎tests/baselines/reference/dynamicImportWithNestedThis_es2015.types‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
=== tests/cases/compiler/dynamicImportWithNestedThis_es2015.ts ===
2+
// https://github.com/Microsoft/TypeScript/issues/17564
3+
class C {
4+
>C : C
5+
6+
private _path = './other';
7+
>_path : string
8+
>'./other' : "./other"
9+
10+
dynamic() {
11+
>dynamic : () => Promise<any>
12+
13+
return import(this._path);
14+
>import(this._path) : Promise<any>
15+
>this._path : string
16+
>this : this
17+
>_path : string
18+
}
19+
}
20+
21+
const c = new C();
22+
>c : C
23+
>new C() : C
24+
>C : typeof C
25+
26+
c.dynamic();
27+
>c.dynamic() : Promise<any>
28+
>c.dynamic : () => Promise<any>
29+
>c : C
30+
>dynamic : () => Promise<any>
31+

‎tests/baselines/reference/dynamicImportWithNestedThis_es5.js‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
//// [dynamicImportWithNestedThis_es5.ts]
2+
// https://github.com/Microsoft/TypeScript/issues/17564
3+
class C {
4+
private _path = './other';
5+
6+
dynamic() {
7+
return import(this._path);
8+
}
9+
}
10+
11+
const c = new C();
12+
c.dynamic();
13+
14+
//// [dynamicImportWithNestedThis_es5.js]
15+
(function (factory) {
16+
if (typeof module === "object" && typeof module.exports === "object") {
17+
var v = factory(require, exports);
18+
if (v !== undefined) module.exports = v;
19+
}
20+
else if (typeof define === "function" && define.amd) {
21+
define(["require", "exports"], factory);
22+
}
23+
})(function (require, exports) {
24+
"use strict";
25+
var __syncRequire = typeof module === "object" && typeof module.exports === "object";
26+
// https://github.com/Microsoft/TypeScript/issues/17564
27+
var C = /** @class */ (function () {
28+
function C() {
29+
this._path = './other';
30+
}
31+
C.prototype.dynamic = function () {
32+
var _this = this;
33+
return __syncRequire ? Promise.resolve().then(function () { return require(_this._path); }) : new Promise(function (resolve_1, reject_1) { require([_this._path], resolve_1, reject_1); });
34+
};
35+
return C;
36+
}());
37+
var c = new C();
38+
c.dynamic();
39+
});

‎tests/baselines/reference/dynamicImportWithNestedThis_es5.symbols‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
=== tests/cases/compiler/dynamicImportWithNestedThis_es5.ts ===
2+
// https://github.com/Microsoft/TypeScript/issues/17564
3+
class C {
4+
>C : Symbol(C, Decl(dynamicImportWithNestedThis_es5.ts, 0, 0))
5+
6+
private _path = './other';
7+
>_path : Symbol(C._path, Decl(dynamicImportWithNestedThis_es5.ts, 1, 9))
8+
9+
dynamic() {
10+
>dynamic : Symbol(C.dynamic, Decl(dynamicImportWithNestedThis_es5.ts, 2, 27))
11+
12+
return import(this._path);
13+
>this._path : Symbol(C._path, Decl(dynamicImportWithNestedThis_es5.ts, 1, 9))
14+
>this : Symbol(C, Decl(dynamicImportWithNestedThis_es5.ts, 0, 0))
15+
>_path : Symbol(C._path, Decl(dynamicImportWithNestedThis_es5.ts, 1, 9))
16+
}
17+
}
18+
19+
const c = new C();
20+
>c : Symbol(c, Decl(dynamicImportWithNestedThis_es5.ts, 9, 5))
21+
>C : Symbol(C, Decl(dynamicImportWithNestedThis_es5.ts, 0, 0))
22+
23+
c.dynamic();
24+
>c.dynamic : Symbol(C.dynamic, Decl(dynamicImportWithNestedThis_es5.ts, 2, 27))
25+
>c : Symbol(c, Decl(dynamicImportWithNestedThis_es5.ts, 9, 5))
26+
>dynamic : Symbol(C.dynamic, Decl(dynamicImportWithNestedThis_es5.ts, 2, 27))
27+

‎tests/baselines/reference/dynamicImportWithNestedThis_es5.types‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
=== tests/cases/compiler/dynamicImportWithNestedThis_es5.ts ===
2+
// https://github.com/Microsoft/TypeScript/issues/17564
3+
class C {
4+
>C : C
5+
6+
private _path = './other';
7+
>_path : string
8+
>'./other' : "./other"
9+
10+
dynamic() {
11+
>dynamic : () => Promise<any>
12+
13+
return import(this._path);
14+
>import(this._path) : Promise<any>
15+
>this._path : string
16+
>this : this
17+
>_path : string
18+
}
19+
}
20+
21+
const c = new C();
22+
>c : C
23+
>new C() : C
24+
>C : typeof C
25+
26+
c.dynamic();
27+
>c.dynamic() : Promise<any>
28+
>c.dynamic : () => Promise<any>
29+
>c : C
30+
>dynamic : () => Promise<any>
31+

‎tests/cases/compiler/dynamicImportWithNestedThis_es2015.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// @lib: es2015
2+
// @target: es2015
3+
// @module: umd
4+
// https://github.com/Microsoft/TypeScript/issues/17564
5+
class C {
6+
private _path = './other';
7+
8+
dynamic() {
9+
return import(this._path);
10+
}
11+
}
12+
13+
const c = new C();
14+
c.dynamic();

‎tests/cases/compiler/dynamicImportWithNestedThis_es5.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// @lib: es2015
2+
// @target: es5
3+
// @module: umd
4+
// https://github.com/Microsoft/TypeScript/issues/17564
5+
class C {
6+
private _path = './other';
7+
8+
dynamic() {
9+
return import(this._path);
10+
}
11+
}
12+
13+
const c = new C();
14+
c.dynamic();

0 commit comments

Comments
 (0)