Skip to content
Permalink
Browse files
fix: avoid downleveled dynamic import closing over specifier expressi…
…on (#49663)

* fix: evaluate dynamic import specifier expressions synchronously

* refactor

* Update src/compiler/transformers/module/module.ts

Co-authored-by: Ron Buckton <ron.buckton@microsoft.com>

* [Experiment]

Co-authored-by: Ron Buckton <ron.buckton@microsoft.com>
  • Loading branch information
Josh-Cena and rbuckton committed Oct 18, 2022
1 parent 11066b2 commit 1f8959f5dc04b2b2c2fc8a7dc53b6ac761e1f754
Show file tree
Hide file tree
Showing 14 changed files with 180 additions and 42 deletions.
@@ -721,7 +721,7 @@ namespace ts {
return createImportCallExpressionUMD(argument ?? factory.createVoidZero(), containsLexicalThis);
case ModuleKind.CommonJS:
default:
return createImportCallExpressionCommonJS(argument, containsLexicalThis);
return createImportCallExpressionCommonJS(argument);
}
}

@@ -745,7 +745,7 @@ namespace ts {
return factory.createConditionalExpression(
/*condition*/ factory.createIdentifier("__syncRequire"),
/*questionToken*/ undefined,
/*whenTrue*/ createImportCallExpressionCommonJS(arg, containsLexicalThis),
/*whenTrue*/ createImportCallExpressionCommonJS(arg),
/*colonToken*/ undefined,
/*whenFalse*/ createImportCallExpressionAMD(argClone, containsLexicalThis)
);
@@ -755,7 +755,7 @@ namespace ts {
return factory.createComma(factory.createAssignment(temp, arg), factory.createConditionalExpression(
/*condition*/ factory.createIdentifier("__syncRequire"),
/*questionToken*/ undefined,
/*whenTrue*/ createImportCallExpressionCommonJS(temp, containsLexicalThis),
/*whenTrue*/ createImportCallExpressionCommonJS(temp, /* isInlineable */ true),
/*colonToken*/ undefined,
/*whenFalse*/ createImportCallExpressionAMD(temp, containsLexicalThis)
));
@@ -820,14 +820,25 @@ namespace ts {
return promise;
}

function createImportCallExpressionCommonJS(arg: Expression | undefined, containsLexicalThis: boolean): Expression {
// import("./blah")
function createImportCallExpressionCommonJS(arg: Expression | undefined, isInlineable?: boolean): Expression {
// import(x)
// emit as
// Promise.resolve().then(function () { return require(x); }) /*CommonJs Require*/
// var _a;
// (_a = x, Promise.resolve().then(() => require(_a)) /*CommonJs Require*/
// We have to wrap require in then callback so that require is done in asynchronously
// if we simply do require in resolve callback in Promise constructor. We will execute the loading immediately
const promiseResolveCall = factory.createCallExpression(factory.createPropertyAccessExpression(factory.createIdentifier("Promise"), "resolve"), /*typeArguments*/ undefined, /*argumentsArray*/ []);
let requireCall: Expression = factory.createCallExpression(factory.createIdentifier("require"), /*typeArguments*/ undefined, arg ? [arg] : []);
// If the arg is not inlineable, we have to evaluate it in the current scope with a temp var
const temp = arg && !isSimpleInlineableExpression(arg) && !isInlineable ? factory.createTempVariable(hoistVariableDeclaration) : undefined;
const promiseResolveCall = factory.createCallExpression(
factory.createPropertyAccessExpression(factory.createIdentifier("Promise"), "resolve"),
/*typeArguments*/ undefined,
/*argumentsArray*/ [],
);
let requireCall: Expression = factory.createCallExpression(
factory.createIdentifier("require"),
/*typeArguments*/ undefined,
temp ? [temp] : arg ? [arg] : [],
);
if (getESModuleInterop(compilerOptions)) {
requireCall = emitHelpers().createImportStarHelper(requireCall);
}
@@ -851,16 +862,11 @@ namespace ts {
/*parameters*/ [],
/*type*/ undefined,
factory.createBlock([factory.createReturnStatement(requireCall)]));

// if there is a lexical 'this' in the import call arguments, ensure we indicate
// that this new function expression indicates it captures 'this' so that the
// es2015 transformer will properly substitute 'this' with '_this'.
if (containsLexicalThis) {
setEmitFlags(func, EmitFlags.CapturesThis);
}
}

return factory.createCallExpression(factory.createPropertyAccessExpression(promiseResolveCall, "then"), /*typeArguments*/ undefined, [func]);
const downleveledImport = factory.createCallExpression(factory.createPropertyAccessExpression(promiseResolveCall, "then"), /*typeArguments*/ undefined, [func]);

return temp === undefined ? downleveledImport : factory.createCommaListExpression([factory.createAssignment(temp, arg!), downleveledImport]);
}

function getHelperExpressionForExport(node: ExportDeclaration, innerExpr: Expression) {
@@ -46,12 +46,13 @@ var __asyncGenerator = (this && this.__asyncGenerator) || function (thisArg, _ar
function foo() {
return __asyncGenerator(this, arguments, function foo_1() {
return __generator(this, function (_a) {
var _b, _c;
switch (_a.label) {
case 0: return [4 /*yield*/, __await("foo")];
case 1: return [4 /*yield*/, _a.sent()];
case 2: return [4 /*yield*/, __await.apply(void 0, [Promise.resolve().then(function () { return require(_a.sent()); })])];
case 2: return [4 /*yield*/, __await.apply(void 0, [(_b = _a.sent(), Promise.resolve().then(function () { return require(_b); }))])];
case 3:
Promise.resolve().then(function () { return require((_a.sent())["default"]); });
_c = (_a.sent())["default"], Promise.resolve().then(function () { return require(_c); });
return [2 /*return*/];
}
});
@@ -0,0 +1,31 @@
//// [dynamicImportEvaluateSpecifier.ts]
// https://github.com/microsoft/TypeScript/issues/48285
let i = 0;

import(String(i++));
import(String(i++));

const getPath = async () => {
/* in reality this would do some async FS operation, or a web request */
return "/root/my/cool/path";
};

const someFunction = async () => {
const result = await import(await getPath());
};


//// [dynamicImportEvaluateSpecifier.js]
var _a, _b;
// https://github.com/microsoft/TypeScript/issues/48285
let i = 0;
_a = String(i++), Promise.resolve().then(() => require(_a));
_b = String(i++), Promise.resolve().then(() => require(_b));
const getPath = async () => {
/* in reality this would do some async FS operation, or a web request */
return "/root/my/cool/path";
};
const someFunction = async () => {
var _a;
const result = await (_a = await getPath(), Promise.resolve().then(() => require(_a)));
};
@@ -0,0 +1,29 @@
=== tests/cases/compiler/dynamicImportEvaluateSpecifier.ts ===
// https://github.com/microsoft/TypeScript/issues/48285
let i = 0;
>i : Symbol(i, Decl(dynamicImportEvaluateSpecifier.ts, 1, 3))

import(String(i++));
>String : Symbol(String, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.core.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --) ... and 3 more)
>i : Symbol(i, Decl(dynamicImportEvaluateSpecifier.ts, 1, 3))

import(String(i++));
>String : Symbol(String, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.core.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --) ... and 3 more)
>i : Symbol(i, Decl(dynamicImportEvaluateSpecifier.ts, 1, 3))

const getPath = async () => {
>getPath : Symbol(getPath, Decl(dynamicImportEvaluateSpecifier.ts, 6, 5))

/* in reality this would do some async FS operation, or a web request */
return "/root/my/cool/path";
};

const someFunction = async () => {
>someFunction : Symbol(someFunction, Decl(dynamicImportEvaluateSpecifier.ts, 11, 5))

const result = await import(await getPath());
>result : Symbol(result, Decl(dynamicImportEvaluateSpecifier.ts, 12, 6))
>getPath : Symbol(getPath, Decl(dynamicImportEvaluateSpecifier.ts, 6, 5))

};

@@ -0,0 +1,44 @@
=== tests/cases/compiler/dynamicImportEvaluateSpecifier.ts ===
// https://github.com/microsoft/TypeScript/issues/48285
let i = 0;
>i : number
>0 : 0

import(String(i++));
>import(String(i++)) : Promise<any>
>String(i++) : string
>String : StringConstructor
>i++ : number
>i : number

import(String(i++));
>import(String(i++)) : Promise<any>
>String(i++) : string
>String : StringConstructor
>i++ : number
>i : number

const getPath = async () => {
>getPath : () => Promise<string>
>async () => { /* in reality this would do some async FS operation, or a web request */ return "/root/my/cool/path";} : () => Promise<string>

/* in reality this would do some async FS operation, or a web request */
return "/root/my/cool/path";
>"/root/my/cool/path" : "/root/my/cool/path"

};

const someFunction = async () => {
>someFunction : () => Promise<void>
>async () => { const result = await import(await getPath());} : () => Promise<void>

const result = await import(await getPath());
>result : any
>await import(await getPath()) : any
>import(await getPath()) : Promise<any>
>await getPath() : string
>getPath() : Promise<string>
>getPath : () => Promise<string>

};

@@ -3,5 +3,6 @@ const path = './foo';
import(path,);

//// [dynamicImportTrailingComma.js]
var _a;
var path = './foo';
Promise.resolve().then(function () { return require(path); });
_a = path, Promise.resolve().then(function () { return require(_a); });
@@ -15,12 +15,14 @@ function returnDynamicLoad(path: string) {
}

//// [importCallExpressionDeclarationEmit1.js]
Promise.resolve().then(() => require(getSpecifier()));
var p0 = Promise.resolve().then(() => require(`${directory}\\${moduleFile}`));
var p1 = Promise.resolve().then(() => require(getSpecifier()));
const p2 = Promise.resolve().then(() => require(whatToLoad ? getSpecifier() : "defaulPath"));
var _a, _b, _c, _d;
_a = getSpecifier(), Promise.resolve().then(() => require(_a));
var p0 = (_b = `${directory}\\${moduleFile}`, Promise.resolve().then(() => require(_b)));
var p1 = (_c = getSpecifier(), Promise.resolve().then(() => require(_c)));
const p2 = (_d = whatToLoad ? getSpecifier() : "defaulPath", Promise.resolve().then(() => require(_d)));
function returnDynamicLoad(path) {
return Promise.resolve().then(() => require(path));
var _a;
return _a = path, Promise.resolve().then(() => require(_a));
}


@@ -10,8 +10,9 @@ const p2 = import();
const p4 = import("pathToModule", "secondModule");

//// [importCallExpressionGrammarError.js]
var _a, _b;
var a = ["./0"];
Promise.resolve().then(() => require(...["PathModule"]));
var p1 = Promise.resolve().then(() => require(...a));
_a = (...["PathModule"]), Promise.resolve().then(() => require(_a));
var p1 = (_b = (...a), Promise.resolve().then(() => require(_b)));
const p2 = Promise.resolve().then(() => require());
const p4 = Promise.resolve().then(() => require("pathToModule"));
@@ -24,6 +24,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
};
function foo() {
return __awaiter(this, void 0, void 0, function* () {
return yield Promise.resolve().then(() => require((yield Promise.resolve().then(() => require("./foo"))).default));
var _a;
return yield (_a = (yield Promise.resolve().then(() => require("./foo"))).default, Promise.resolve().then(() => require(_a)));
});
}
@@ -52,9 +52,10 @@ var __generator = (this && this.__generator) || function (thisArg, body) {
function foo() {
return __awaiter(this, void 0, void 0, function () {
return __generator(this, function (_a) {
var _b;
switch (_a.label) {
case 0: return [4 /*yield*/, Promise.resolve().then(function () { return require("./foo"); })];
case 1: return [4 /*yield*/, Promise.resolve().then(function () { return require((_a.sent()).default); })];
case 1: return [4 /*yield*/, (_b = (_a.sent()).default, Promise.resolve().then(function () { return require(_b); }))];
case 2: return [2 /*return*/, _a.sent()];
}
});
@@ -42,21 +42,23 @@ class C {
exports.C = C;
//// [1.js]
"use strict";
var _a, _b, _c, _d, _e, _f, _g;
Object.defineProperty(exports, "__esModule", { value: true });
Promise.resolve().then(() => require(`${directory}\\${moduleFile}`));
Promise.resolve().then(() => require(getSpecifier()));
var p1 = Promise.resolve().then(() => require(ValidSomeCondition() ? "./0" : "externalModule"));
var p1 = Promise.resolve().then(() => require(getSpecifier()));
var p11 = Promise.resolve().then(() => require(getSpecifier()));
const p2 = Promise.resolve().then(() => require(whatToLoad ? getSpecifier() : "defaulPath"));
_a = `${directory}\\${moduleFile}`, Promise.resolve().then(() => require(_a));
_b = getSpecifier(), Promise.resolve().then(() => require(_b));
var p1 = (_c = ValidSomeCondition() ? "./0" : "externalModule", Promise.resolve().then(() => require(_c)));
var p1 = (_d = getSpecifier(), Promise.resolve().then(() => require(_d)));
var p11 = (_e = getSpecifier(), Promise.resolve().then(() => require(_e)));
const p2 = (_f = whatToLoad ? getSpecifier() : "defaulPath", Promise.resolve().then(() => require(_f)));
p1.then(zero => {
return zero.foo(); // ok, zero is any
});
let j;
var p3 = Promise.resolve().then(() => require(j = getSpecifier()));
var p3 = (_g = j = getSpecifier(), Promise.resolve().then(() => require(_g)));
function* loadModule(directories) {
var _a;
for (const directory of directories) {
const path = `${directory}\\moduleFile`;
Promise.resolve().then(() => require(yield path));
_a = yield path, Promise.resolve().then(() => require(_a));
}
}
@@ -14,12 +14,13 @@ var p3 = import(["path1", "path2"]);
var p4 = import(()=>"PathToModule");

//// [importCallExpressionSpecifierNotStringTypeError.js]
var _a, _b, _c, _d, _e;
// Error specifier is not assignable to string
Promise.resolve().then(() => require(getSpecifier()));
var p1 = Promise.resolve().then(() => require(getSpecifier()));
const p2 = Promise.resolve().then(() => require(whatToLoad ? getSpecifier() : "defaulPath"));
_a = getSpecifier(), Promise.resolve().then(() => require(_a));
var p1 = (_b = getSpecifier(), Promise.resolve().then(() => require(_b)));
const p2 = (_c = whatToLoad ? getSpecifier() : "defaulPath", Promise.resolve().then(() => require(_c)));
p1.then(zero => {
return zero.foo(); // ok, zero is any
});
var p3 = Promise.resolve().then(() => require(["path1", "path2"]));
var p4 = Promise.resolve().then(() => require(() => "PathToModule"));
var p3 = (_d = ["path1", "path2"], Promise.resolve().then(() => require(_d)));
var p4 = (_e = () => "PathToModule", Promise.resolve().then(() => require(_e)));
@@ -58,6 +58,7 @@ var v = import(String());


//// [jsdocInTypeScript.js]
var _a;
var T = /** @class */ (function () {
function T() {
}
@@ -92,4 +93,4 @@ var E = {};
E[""];
// make sure import types in JSDoc are not resolved
/** @type {import("should-not-be-resolved").Type} */
var v = Promise.resolve().then(function () { return require(String()); });
var v = (_a = String(), Promise.resolve().then(function () { return require(_a); }));
@@ -0,0 +1,17 @@
// @lib: es2019
// @target: es2019
// @module: commonjs
// https://github.com/microsoft/TypeScript/issues/48285
let i = 0;

import(String(i++));
import(String(i++));

const getPath = async () => {
/* in reality this would do some async FS operation, or a web request */
return "/root/my/cool/path";
};

const someFunction = async () => {
const result = await import(await getPath());
};

0 comments on commit 1f8959f

Please sign in to comment.