Skip to content

fix: allow exporting TSDeclareFunction as default#14763

Merged
nicolo-ribaudo merged 3 commits intobabel:mainfrom
zxbodya:fix-export-default-ts-declare-function
Jul 18, 2022
Merged

fix: allow exporting TSDeclareFunction as default#14763
nicolo-ribaudo merged 3 commits intobabel:mainfrom
zxbodya:fix-export-default-ts-declare-function

Conversation

@zxbodya
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

liuxingbaoyu commented Jul 16, 2022

image

It looks like this.

playground

@zxbodya
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

5 participants