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.

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

fix: avoid downleveled dynamic import closing over specifier expression #49663

Merged
merged 4 commits into from Oct 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 22 additions & 16 deletions src/compiler/transformers/module/module.ts
Expand Up @@ -721,7 +721,7 @@ namespace ts {
return createImportCallExpressionUMD(argument ?? factory.createVoidZero(), containsLexicalThis);
case ModuleKind.CommonJS:
default:
return createImportCallExpressionCommonJS(argument, containsLexicalThis);
return createImportCallExpressionCommonJS(argument);
}
}

Expand All @@ -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)
);
Expand All @@ -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)
));
Expand Down Expand Up @@ -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);
}
Expand All @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions tests/baselines/reference/asyncImportNestedYield.js
Expand Up @@ -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*/];
}
});
Expand Down
31 changes: 31 additions & 0 deletions tests/baselines/reference/dynamicImportEvaluateSpecifier.js
@@ -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)));
};
29 changes: 29 additions & 0 deletions tests/baselines/reference/dynamicImportEvaluateSpecifier.symbols
@@ -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))

};

44 changes: 44 additions & 0 deletions tests/baselines/reference/dynamicImportEvaluateSpecifier.types
@@ -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 changes: 2 additions & 1 deletion tests/baselines/reference/dynamicImportTrailingComma.js
Expand Up @@ -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); });
Expand Up @@ -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));
}


Expand Down
5 changes: 3 additions & 2 deletions tests/baselines/reference/importCallExpressionGrammarError.js
Expand Up @@ -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)));
Comment on lines +15 to +16
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is invalid grammar, but the input is invalid grammar as well. I assume we can safely treat it as garbage in, garbage out?

const p2 = Promise.resolve().then(() => require());
const p4 = Promise.resolve().then(() => require("pathToModule"));
3 changes: 2 additions & 1 deletion tests/baselines/reference/importCallExpressionNestedCJS.js
Expand Up @@ -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)));
});
}
3 changes: 2 additions & 1 deletion tests/baselines/reference/importCallExpressionNestedCJS2.js
Expand Up @@ -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()];
}
});
Expand Down
Expand Up @@ -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));
}
}
Expand Up @@ -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)));
3 changes: 2 additions & 1 deletion tests/baselines/reference/jsdocInTypeScript.js
Expand Up @@ -58,6 +58,7 @@ var v = import(String());


//// [jsdocInTypeScript.js]
var _a;
var T = /** @class */ (function () {
function T() {
}
Expand Down Expand Up @@ -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); }));
17 changes: 17 additions & 0 deletions tests/cases/compiler/dynamicImportEvaluateSpecifier.ts
@@ -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());
};