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

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

Closed
Knagis opened this issue May 8, 2024 · 7 comments Β· Fixed by #58489
Closed

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

Knagis opened this issue May 8, 2024 · 7 comments Β· 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.

@Knagis
Copy link
Contributor Author

Knagis commented May 28, 2024

@jakebailey is there a chance the fix for this could be merged before 5.5.1?

@jakebailey
Copy link
Member

What version of TS is the above output from? Is that expected output? Unexpected output?

That output is assigning mockedClassComponent to the exports, not omitting it entirely, so I don't think what you've described is the same thing (and shows that the current state is working).

@meriouma
Copy link

Sorry, I think I was tracking the wrong issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants