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: allow exporting TSDeclareFunction as default #14763

Merged

Conversation

zxbodya
Copy link
Contributor

@zxbodya zxbodya commented Jul 16, 2022

Q                       A
Fixed Issues?
Patch: Bug Fix? Yes
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

Updating @babel/types to allow TSDeclareFunction in default export.
It was working earlier, but stopped somewhere in between 7.18.2 and current (7.18.8) version

@babel-bot
Copy link
Collaborator

babel-bot commented Jul 16, 2022

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/52535/

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Jul 16, 2022

What is the corresponding TS syntax? All of these error in the TS playground 🤔

declare export default function f(): any; // "export" modifier must precede "declare" modifier
export declare default function f(): any; // lots of parsing errors
export default declare function f(): any; // expects a ; after declare

@zxbodya
Copy link
Contributor Author

zxbodya commented Jul 16, 2022

example with the syntax(playground):

export default function test(a: number): boolean;
export default function test(a: boolean): boolean;
export default function test(a: any): boolean {
    return false;
};

@liuxingbaoyu
Copy link
Member

liuxingbaoyu commented Jul 16, 2022

image

It looks like this.

playground

@zxbodya
Copy link
Contributor Author

zxbodya commented Jul 16, 2022

@liuxingbaoyu here is it in astexplorer (for babel parser it is to be ExportDefaultDeclaration with TSDeclareFunction in a declaration field)

@liuxingbaoyu
Copy link
Member

Related to this.
I tested in @babel/parser 7.18.8 which produces the ExportDefaultDeclaration node.

@liuxingbaoyu liuxingbaoyu added PR: Bug Fix 🐛 A type of pull request used for our changelog categories pkg: types area: typescript i: regression labels Jul 16, 2022
Comment on lines +439 to 440
// @ts-expect-error TSDeclareFunction is not expected here
path.replaceWith(buildExportCall("default", declar));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

from my understanding TSDeclareFunction is not expected to get there(I would assume it is to be removed somewhere earlier, and it is fine to ignore this)

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

Can you add a test so that we don't accidentally break this again?

@nicolo-ribaudo nicolo-ribaudo changed the title fix: update babel-types to allow exporting TSDeclareFunction as default fix: allow exporting TSDeclareFunction as default Jul 18, 2022
@nicolo-ribaudo nicolo-ribaudo merged commit d5f4505 into babel:main Jul 18, 2022
@nicolo-ribaudo nicolo-ribaudo deleted the fix-export-default-ts-declare-function branch July 18, 2022 08:47
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 18, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: typescript i: regression outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: types PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants