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.

Sign up for GitHub

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

Jump to bottom

5.5.0 regression - Named exports of functions are not emitted for CommonJS #58473

Open
Knagis opened this issue May 8, 2024 · 4 comments · May be fixed by #58489
Open

5.5.0 regression - Named exports of functions are not emitted for CommonJS #58473

Knagis opened this issue May 8, 2024 · 4 comments · May be fixed by #58489

Comments

@Knagis
Copy link
Contributor

Knagis commented May 8, 2024

🔎 Search Terms

commonjs export

🕗 Version & Regression Information

  • This changed between versions 5.4.5 and 5.5.0

⏯ Playground Link

https://www.typescriptlang.org/play/?target=7&module=1&isolatedModules=true&verbatimModuleSyntax=false&ts=5.5.0-beta#code/GYVwdgxgLglg9mABMOcAUBKAXIgbnGAE0QG8AoAXzLIFMAPABzgCcpSzFPlUAaSgbjJA

💻 Code

function foo(): void {
}

export {
    foo,
};

🙁 Actual behavior

ts 5.5.0-beta and 5.5.0-dev-20240508 both emit:

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.foo = void 0;
function foo() {
}

note missing last line

🙂 Expected behavior

ts 5.4.5 emit

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.foo = void 0;
function foo() {
}
exports.foo = foo;

Additional information about the issue

No response
@Knagis Knagis changed the title Named exports of functions are not emitted for CommonJS 5.5.0 regression - Named exports of functions are not emitted for CommonJS May 8, 2024
@Knagis
Copy link
Contributor Author

Knagis commented May 8, 2024

the problem shows up only with functions. exporting classes or variables via export { foo } work fine. also export function foo() {} works fine.

@jakebailey
Copy link
Member

Most likely this was #57669 (you could try using https://www.npmjs.com/package/every-ts to bisect and verify).

@Knagis
Copy link
Contributor Author

Knagis commented May 9, 2024

Bisect confirms that it is the cause

881f449a8a10839db5d535c3a407621d53c78666 is the first bad commit
commit 881f449a8a10839db5d535c3a407621d53c78666
Author: Ron Buckton
Date:   Wed Mar 6 18:43:59 2024 -0500

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

@jakebailey
Copy link
Member

Yeah, the issue appears to be that collectExternalModuleInfo doesn't do the same thing it does for function declarations when processing export declarations.

@jakebailey jakebailey linked a pull request May 9, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Handle exported functions in ExportDeclarations for CJS/AMD/UMD emit jakebailey/TypeScript
2 participants
@jakebailey @Knagis