Skip to content

Hoist function exports to top of module body in CJS/AMD/UMD #57669

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

Merged
merged 2 commits into from
Mar 6, 2024
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.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
20 changes: 15 additions & 5 deletions src/compiler/transformers/module/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,21 +264,26 @@ export function transformModule(context: TransformationContext): (x: SourceFile
if (shouldEmitUnderscoreUnderscoreESModule()) {
append(statements, createUnderscoreUnderscoreESModule());
}
if (length(currentModuleInfo.exportedNames)) {
if (some(currentModuleInfo.exportedNames)) {
const chunkSize = 50;
for (let i = 0; i < currentModuleInfo.exportedNames!.length; i += chunkSize) {
for (let i = 0; i < currentModuleInfo.exportedNames.length; i += chunkSize) {
append(
statements,
factory.createExpressionStatement(
reduceLeft(
currentModuleInfo.exportedNames!.slice(i, i + chunkSize),
currentModuleInfo.exportedNames.slice(i, i + chunkSize),
(prev, nextId) => factory.createAssignment(factory.createPropertyAccessExpression(factory.createIdentifier("exports"), factory.createIdentifier(idText(nextId))), prev),
factory.createVoidZero() as Expression,
),
),
);
}
}
if (some(currentModuleInfo.exportedFunctions)) {
for (const f of currentModuleInfo.exportedFunctions) {
appendExportsOfHoistedDeclaration(statements, f);
}
}

append(statements, visitNode(currentModuleInfo.externalHelpersImportDeclaration, topLevelVisitor, isStatement));
addRange(statements, visitNodes(node.statements, topLevelVisitor, isStatement, statementOffset));
Expand Down Expand Up @@ -601,9 +606,14 @@ export function transformModule(context: TransformationContext): (x: SourceFile
if (shouldEmitUnderscoreUnderscoreESModule()) {
append(statements, createUnderscoreUnderscoreESModule());
}
if (length(currentModuleInfo.exportedNames)) {
if (some(currentModuleInfo.exportedNames)) {
append(statements, factory.createExpressionStatement(reduceLeft(currentModuleInfo.exportedNames, (prev, nextId) => factory.createAssignment(factory.createPropertyAccessExpression(factory.createIdentifier("exports"), factory.createIdentifier(idText(nextId))), prev), factory.createVoidZero() as Expression)));
}
if (some(currentModuleInfo.exportedFunctions)) {
for (const f of currentModuleInfo.exportedFunctions) {
appendExportsOfHoistedDeclaration(statements, f);
}
}

// Visit each statement of the module body.
append(statements, visitNode(currentModuleInfo.externalHelpersImportDeclaration, topLevelVisitor, isStatement));
Expand Down Expand Up @@ -1704,7 +1714,7 @@ export function transformModule(context: TransformationContext): (x: SourceFile
statements = append(statements, visitEachChild(node, visitor, context));
}

statements = appendExportsOfHoistedDeclaration(statements, node);
// NOTE: CommonJS/AMD/UMD exports are hoisted to the top of the module body and do not need to be added here.
return singleOrMany(statements);
}

Expand Down
19 changes: 18 additions & 1 deletion src/compiler/transformers/module/system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ export function transformSystemModule(context: TransformationContext): (x: Sourc
// this set is used to filter names brought by star expors.

// local names set should only be added if we have anything exported
if (!moduleInfo.exportedNames && moduleInfo.exportSpecifiers.size === 0) {
if (!some(moduleInfo.exportedNames) && !some(moduleInfo.exportedFunctions) && moduleInfo.exportSpecifiers.size === 0) {
// no exported declarations (export var ...) or export specifiers (export {x})
// check if we have any non star export declarations.
let hasExportDeclarationWithExportClause = false;
Expand Down Expand Up @@ -469,6 +469,23 @@ export function transformSystemModule(context: TransformationContext): (x: Sourc
}
}

if (moduleInfo.exportedFunctions) {
for (const f of moduleInfo.exportedFunctions) {
if (hasSyntacticModifier(f, ModifierFlags.Default)) {
continue;
}
Debug.assert(!!f.name);

// write name of exported declaration, i.e 'export var x...'
exportedNames.push(
factory.createPropertyAssignment(
factory.createStringLiteralFromNode(f.name),
factory.createTrue(),
),
);
}
}

const exportedNamesStorageRef = factory.createUniqueName("exportedNames");
statements.push(
factory.createVariableStatement(
Expand Down
8 changes: 5 additions & 3 deletions src/compiler/transformers/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ export interface ExternalModuleInfo {
externalHelpersImportDeclaration: ImportDeclaration | undefined; // import of external helpers
exportSpecifiers: IdentifierNameMap<ExportSpecifier[]>; // file-local export specifiers by name (no reexports)
exportedBindings: Identifier[][]; // exported names of local declarations
exportedNames: Identifier[] | undefined; // all exported names in the module, both local and reexported
exportedNames: Identifier[] | undefined; // all exported names in the module, both local and reexported, excluding the names of locally exported function declarations
exportedFunctions: FunctionDeclaration[] | undefined; // all of the top-level exported function declarations
exportEquals: ExportAssignment | undefined; // an export= declaration if one was present
hasExportStarsToExportValues: boolean; // whether this module contains export*
}
Expand Down Expand Up @@ -171,6 +172,7 @@ export function collectExternalModuleInfo(context: TransformationContext, source
const exportedBindings: Identifier[][] = [];
const uniqueExports = new Map<string, boolean>();
let exportedNames: Identifier[] | undefined;
let exportedFunctions: FunctionDeclaration[] | undefined;
let hasExportDefault = false;
let exportEquals: ExportAssignment | undefined;
let hasExportStarsToExportValues = false;
Expand Down Expand Up @@ -250,6 +252,7 @@ export function collectExternalModuleInfo(context: TransformationContext, source

case SyntaxKind.FunctionDeclaration:
if (hasSyntacticModifier(node, ModifierFlags.Export)) {
exportedFunctions = append(exportedFunctions, node as FunctionDeclaration);
if (hasSyntacticModifier(node, ModifierFlags.Default)) {
// export default function() { }
if (!hasExportDefault) {
Expand All @@ -263,7 +266,6 @@ export function collectExternalModuleInfo(context: TransformationContext, source
if (!uniqueExports.get(idText(name))) {
multiMapSparseArrayAdd(exportedBindings, getOriginalNodeId(node), name);
uniqueExports.set(idText(name), true);
exportedNames = append(exportedNames, name);
}
}
}
Expand Down Expand Up @@ -297,7 +299,7 @@ export function collectExternalModuleInfo(context: TransformationContext, source
externalImports.unshift(externalHelpersImportDeclaration);
}

return { externalImports, exportSpecifiers, exportEquals, hasExportStarsToExportValues, exportedBindings, exportedNames, externalHelpersImportDeclaration };
return { externalImports, exportSpecifiers, exportEquals, hasExportStarsToExportValues, exportedBindings, exportedNames, exportedFunctions, externalHelpersImportDeclaration };

function addExportedNamesForExportDeclaration(node: ExportDeclaration) {
for (const specifier of cast(node.exportClause, isNamedExports).elements) {
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/APISample_compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ compile(process.argv.slice(2), {
* Please log a "breaking change" issue for any API breaking change affecting this issue
*/
Object.defineProperty(exports, "__esModule", { value: true });
exports.compile = void 0;
exports.compile = compile;
var ts = require("typescript");
function compile(fileNames, options) {
var program = ts.createProgram(fileNames, options);
Expand All @@ -73,7 +73,6 @@ function compile(fileNames, options) {
console.log("Process exiting with code '".concat(exitCode, "'."));
process.exit(exitCode);
}
exports.compile = compile;
compile(process.argv.slice(2), {
noEmitOnError: true, noImplicitAny: true,
target: ts.ScriptTarget.ES5, module: ts.ModuleKind.CommonJS
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/APISample_linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ fileNames.forEach(fileName => {
* Please log a "breaking change" issue for any API breaking change affecting this issue
*/
Object.defineProperty(exports, "__esModule", { value: true });
exports.delint = void 0;
exports.delint = delint;
var ts = require("typescript");
function delint(sourceFile) {
delintNode(sourceFile);
Expand Down Expand Up @@ -119,7 +119,6 @@ function delint(sourceFile) {
console.log("".concat(sourceFile.fileName, " (").concat(line + 1, ",").concat(character + 1, "): ").concat(message));
}
}
exports.delint = delint;
var fileNames = process.argv.slice(2);
fileNames.forEach(function (fileName) {
// Parse a file
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/APISample_parseConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export function createProgram(rootFiles: string[], compilerOptionsJson: string):
* Please log a "breaking change" issue for any API breaking change affecting this issue
*/
Object.defineProperty(exports, "__esModule", { value: true });
exports.createProgram = void 0;
exports.createProgram = createProgram;
var ts = require("typescript");
function printError(error) {
if (!error) {
Expand All @@ -77,4 +77,3 @@ function createProgram(rootFiles, compilerOptionsJson) {
}
return ts.createProgram(rootFiles, settings.options);
}
exports.createProgram = createProgram;
3 changes: 1 addition & 2 deletions tests/baselines/reference/aliasUsedAsNameValue.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ exports.id = void 0;
//// [aliasUsedAsNameValue_1.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.b = void 0;
function b(a) { return null; }
exports.b = b;
function b(a) { return null; }
//// [aliasUsedAsNameValue_2.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ var __extends = (this && this.__extends) || (function () {
define("Configurable", ["require", "exports"], function (require, exports) {
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.Configurable = void 0;
exports.Configurable = Configurable;
function Configurable(base) {
return /** @class */ (function (_super) {
__extends(class_1, _super);
Expand All @@ -54,7 +54,6 @@ define("Configurable", ["require", "exports"], function (require, exports) {
return class_1;
}(base));
}
exports.Configurable = Configurable;
});
define("Class", ["require", "exports", "Configurable"], function (require, exports, Configurable_1) {
"use strict";
Expand Down
5 changes: 2 additions & 3 deletions tests/baselines/reference/anonClassDeclarationEmitIsAnon.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ var __extends = (this && this.__extends) || (function () {
};
})();
Object.defineProperty(exports, "__esModule", { value: true });
exports.Timestamped = exports.wrapClass = void 0;
exports.wrapClass = wrapClass;
exports.Timestamped = Timestamped;
function wrapClass(param) {
return /** @class */ (function () {
function Wrapped() {
Expand All @@ -63,7 +64,6 @@ function wrapClass(param) {
return Wrapped;
}());
}
exports.wrapClass = wrapClass;
function Timestamped(Base) {
return /** @class */ (function (_super) {
__extends(class_1, _super);
Expand All @@ -75,7 +75,6 @@ function Timestamped(Base) {
return class_1;
}(Base));
}
exports.Timestamped = Timestamped;
//// [index.js]
"use strict";
var __extends = (this && this.__extends) || (function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ var __extends = (this && this.__extends) || (function () {
};
})();
Object.defineProperty(exports, "__esModule", { value: true });
exports.y = exports.X = void 0;
exports.X = void 0;
exports.y = y;
var X = /** @class */ (function () {
function X(a) {
this.a = a;
Expand All @@ -44,7 +45,6 @@ function y() {
return class_1;
}(X));
}
exports.y = y;


//// [anonymousClassDeclarationDoesntPrintWithReadonly.d.ts]
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/anonymousDefaultExportsAmd.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ define(["require", "exports"], function (require, exports) {
define(["require", "exports"], function (require, exports) {
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
function default_1() { }
exports.default = default_1;
function default_1() { }
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ exports.default = default_1;
//// [b.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
function default_1() { }
exports.default = default_1;
function default_1() { }
2 changes: 1 addition & 1 deletion tests/baselines/reference/anonymousDefaultExportsUmd.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ export default function() {}
})(function (require, exports) {
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
function default_1() { }
exports.default = default_1;
function default_1() { }
});
3 changes: 1 addition & 2 deletions tests/baselines/reference/arrayDestructuringInSwitch1.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export function evaluate(expression: Expression): boolean {
//// [arrayDestructuringInSwitch1.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.evaluate = void 0;
exports.evaluate = evaluate;
function evaluate(expression) {
if (Array.isArray(expression)) {
var operator = expression[0], operands = expression.slice(1);
Expand All @@ -46,4 +46,3 @@ function evaluate(expression) {
return expression === 'true';
}
}
exports.evaluate = evaluate;
3 changes: 1 addition & 2 deletions tests/baselines/reference/asOperator4.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ import { foo } from './foo';
//// [foo.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.foo = void 0;
function foo() { }
exports.foo = foo;
function foo() { }
//// [bar.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ function isNonNullable(obj) {
throw new Error("Must not be a nullable value");
}
}
exports.isNonNullable = isNonNullable;
//// [test.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
Expand Down
5 changes: 2 additions & 3 deletions tests/baselines/reference/baseConstraintOfDecorator.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ var __extends = (this && this.__extends) || (function () {
};
})();
Object.defineProperty(exports, "__esModule", { value: true });
exports.classExtender2 = exports.classExtender = void 0;
exports.classExtender = classExtender;
exports.classExtender2 = classExtender2;
function classExtender(superClass, _instanceModifier) {
return /** @class */ (function (_super) {
__extends(decoratorFunc, _super);
Expand All @@ -55,7 +56,6 @@ function classExtender(superClass, _instanceModifier) {
return decoratorFunc;
}(superClass));
}
exports.classExtender = classExtender;
var MyClass = /** @class */ (function () {
function MyClass() {
}
Expand All @@ -76,4 +76,3 @@ function classExtender2(superClass, _instanceModifier) {
return decoratorFunc;
}(superClass));
}
exports.classExtender2 = classExtender2;
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,16 @@ var __importDefault = (this && this.__importDefault) || function (mod) {
return (mod && mod.__esModule) ? mod : { "default": mod };
};
Object.defineProperty(exports, "__esModule", { value: true });
exports.HereIsTheError = exports.ComponentWithUnion = void 0;
exports.ComponentWithUnion = ComponentWithUnion;
exports.HereIsTheError = HereIsTheError;
var react_1 = __importDefault(require("react"));
function ComponentWithUnion(props) {
return react_1.default.createElement("h1", null);
}
exports.ComponentWithUnion = ComponentWithUnion;
// Usage with React tsx
function HereIsTheError() {
return (react_1.default.createElement(ComponentWithUnion, { multi: false, value: 's', onChange: function (val) { return console.log(val); } }));
}
exports.HereIsTheError = HereIsTheError;
// Usage with pure TypeScript
ComponentWithUnion({
multi: false,
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/circularReferenceInImport.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@ export function foo() {
//// [app.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.foo = void 0;
exports.foo = foo;
function foo() {
return new Object();
}
exports.foo = foo;


//// [app.d.ts]
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/circularResolvedSignature.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,11 @@ export function Component() {
//// [circularResolvedSignature.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.Component = void 0;
exports.Component = Component;
function Component() {
var _a = useState(function () { return ({
value: "string", // this should be a number
foo: function (arg) { return setState(arg); },
bar: function (arg) { return setState(arg); },
}); }), state = _a[0], setState = _a[1];
}
exports.Component = Component;
Loading