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

Return rejected promise when stringify import specifier throws #15290

Merged
merged 6 commits into from Dec 23, 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
47 changes: 47 additions & 0 deletions packages/babel-helper-module-transforms/src/dynamic-import.ts
Expand Up @@ -4,6 +4,7 @@
import * as t from "@babel/types";
import template from "@babel/template";

// TODO(Babel 8): Remove this
SuperSodaSea marked this conversation as resolved.
Show resolved Hide resolved
export function getDynamicImportSource(
node: t.CallExpression,
): t.StringLiteral | t.TemplateLiteral {
SuperSodaSea marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -13,3 +14,49 @@ export function getDynamicImportSource(
? source
: (template.expression.ast`\`\${${source}}\`` as t.TemplateLiteral);
}

export function buildDynamicImport(
node: t.CallExpression,
deferToThen: boolean,
wrapWithPromise: boolean,
builder: (specifier: t.Expression) => t.Expression,
): t.Expression {
const [specifier] = node.arguments;

if (
t.isStringLiteral(specifier) ||
(t.isTemplateLiteral(specifier) && specifier.quasis.length === 0)
) {
if (deferToThen) {
return template.expression.ast`
Promise.resolve().then(() => ${builder(specifier)})
`;
} else return builder(specifier);
}

const specifierToString = t.isTemplateLiteral(specifier)
? t.identifier("specifier")
: t.templateLiteral(
[t.templateElement({ raw: "" }), t.templateElement({ raw: "" })],
[t.identifier("specifier")],
);

if (deferToThen) {
return template.expression.ast`
(specifier =>
new Promise(r => r(${specifierToString}))
.then(s => ${builder(t.identifier("s"))})
)(${specifier})
`;
} else if (wrapWithPromise) {
return template.expression.ast`
(specifier =>
new Promise(r => r(${builder(specifierToString)}))
)(${specifier})
`;
} else {
return template.expression.ast`
(specifier => ${builder(specifierToString)})(${specifier})
`;
}
}
2 changes: 1 addition & 1 deletion packages/babel-helper-module-transforms/src/index.ts
Expand Up @@ -35,7 +35,7 @@ import type {
} from "./normalize-and-load-metadata";
import type { NodePath } from "@babel/traverse";

export { getDynamicImportSource } from "./dynamic-import";
export { buildDynamicImport, getDynamicImportSource } from "./dynamic-import";

export { default as getModuleName } from "./get-module-name";
export type { PluginOptions } from "./get-module-name";
Expand Down
@@ -0,0 +1 @@
import(2);
@@ -0,0 +1,3 @@
define(["require"], function (_require) {
(specifier => new Promise((_resolve, _reject) => _require([`${specifier}`], imported => _resolve(babelHelpers.interopRequireWildcard(imported)), _reject)))(2);
});
@@ -0,0 +1 @@
module.exports = 1;
@@ -0,0 +1,3 @@
let x = 1;
return expect(import(`./${x}.js`))
.resolves.toHaveProperty("default", 1);
@@ -0,0 +1,5 @@
{
"parserOpts": {
"allowReturnOutsideFunction": true
}
}
@@ -0,0 +1,11 @@
expect(() => {
function f() { throw "should throw"; }
import(f());
}).toThrow("should throw");

expect(() => {
const a = {
get x() { throw "should throw"; },
};
import(a.x);
}).toThrow("should throw");
@@ -0,0 +1,6 @@
return expect(import({
[Symbol.toPrimitive](hint) {
if (hint === "string") return "./foo.js";
return null;
}
})).resolves.toHaveProperty("default", "foo");
@@ -0,0 +1 @@
module.exports = "foo";
@@ -0,0 +1,5 @@
{
"parserOpts": {
"allowReturnOutsideFunction": true
}
}
@@ -0,0 +1 @@
module.exports = 1;
@@ -0,0 +1,7 @@
let i = 0;
const promise = expect(import({ toString: () => `./${++i}.js` }))
.resolves.toHaveProperty("default", 1);
expect(i).toBe(1);
++i;
expect(i).toBe(2);
return promise;
@@ -0,0 +1,5 @@
{
"parserOpts": {
"allowReturnOutsideFunction": true
}
}
@@ -0,0 +1,2 @@
return expect(import({ toString: () => { throw "toString failed"; } }))
.rejects.toBe("toString failed");
@@ -0,0 +1,5 @@
{
"parserOpts": {
"allowReturnOutsideFunction": true
}
}
@@ -0,0 +1,2 @@
const x = 1;
import(`./${x}.js`);
@@ -0,0 +1,2 @@
const x = 1;
(specifier => new Promise(r => r(specifier)).then(s => babelHelpers.interopRequireWildcard(require(s))))(`./${x}.js`);
@@ -1 +1 @@
Promise.resolve(`${2}`).then(s => babelHelpers.interopRequireWildcard(require(s)));
(specifier => new Promise(r => r(`${specifier}`)).then(s => babelHelpers.interopRequireWildcard(require(s))))(2);
@@ -0,0 +1 @@
import(2);
@@ -0,0 +1,10 @@
System.register([], function (_export, _context) {
"use strict";

return {
setters: [],
execute: function () {
(specifier => new Promise(r => r(_context.import(`${specifier}`))))(2);
}
};
});
22 changes: 14 additions & 8 deletions packages/babel-plugin-transform-modules-amd/src/index.ts
@@ -1,5 +1,6 @@
import { declare } from "@babel/helper-plugin-utils";
import {
buildDynamicImport,
isModule,
rewriteModuleStatementsAndPrepareHeader,
type RewriteModuleStatementsAndPrepareHeaderOptions,
Expand All @@ -9,7 +10,6 @@ import {
ensureStatementsHoisted,
wrapInterop,
getModuleName,
getDynamicImportSource,
} from "@babel/helper-module-transforms";
import { template, types as t } from "@babel/core";
import type { PluginOptions } from "@babel/helper-module-transforms";
Expand Down Expand Up @@ -99,14 +99,20 @@ export default declare<State>((api, options: Options) => {
if (!noInterop) result = wrapInterop(path, result, "namespace");

path.replaceWith(
template.expression.ast`
new Promise((${resolveId}, ${rejectId}) =>
${requireId}(
[${getDynamicImportSource(path.node)}],
imported => ${t.cloneNode(resolveId)}(${result}),
${t.cloneNode(rejectId)}
buildDynamicImport(
path.node,
false,
false,
specifier => template.expression.ast`
new Promise((${resolveId}, ${rejectId}) =>
${requireId}(
[${specifier}],
imported => ${t.cloneNode(resolveId)}(${result}),
${t.cloneNode(rejectId)}
)
)
)`,
`,
),
);
},

Expand Down
Expand Up @@ -3,7 +3,7 @@

import type { NodePath } from "@babel/traverse";
import { types as t, template, type File } from "@babel/core";
import { getDynamicImportSource } from "@babel/helper-module-transforms";
import { buildDynamicImport } from "@babel/helper-module-transforms";

const requireNoInterop = (source: t.Expression) =>
template.expression.ast`require(${source})`;
Expand All @@ -20,19 +20,9 @@ export function transformDynamicImport(
) {
const buildRequire = noInterop ? requireNoInterop : requireInterop;

const source = getDynamicImportSource(path.node);

const replacement =
t.isStringLiteral(source) ||
(t.isTemplateLiteral(source) && source.quasis.length === 0)
? template.expression.ast`
Promise.resolve().then(() => ${buildRequire(source, file)})
`
: template.expression.ast`
Promise.resolve(${source}).then(
s => ${buildRequire(t.identifier("s"), file)}
)
`;

path.replaceWith(replacement);
path.replaceWith(
buildDynamicImport(path.node, true, false, specifier =>
buildRequire(specifier, file),
),
);
}
16 changes: 9 additions & 7 deletions packages/babel-plugin-transform-modules-systemjs/src/index.ts
Expand Up @@ -2,9 +2,9 @@ import { declare } from "@babel/helper-plugin-utils";
import hoistVariables from "@babel/helper-hoist-variables";
import { template, types as t } from "@babel/core";
import {
rewriteThis,
buildDynamicImport,
getModuleName,
getDynamicImportSource,
rewriteThis,
} from "@babel/helper-module-transforms";
import type { PluginOptions } from "@babel/helper-module-transforms";
import { isIdentifierName } from "@babel/helper-validator-identifier";
Expand Down Expand Up @@ -274,12 +274,14 @@ export default declare<PluginState>((api, options: Options) => {
}

path.replaceWith(
t.callExpression(
t.memberExpression(
t.identifier(state.contextIdent),
t.identifier("import"),
buildDynamicImport(path.node, false, true, specifier =>
t.callExpression(
t.memberExpression(
t.identifier(state.contextIdent),
t.identifier("import"),
),
[specifier],
),
[getDynamicImportSource(path.node)],
),
);
}
Expand Down