Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_js_parser): export default function in declare module #3485 #3775

Merged
merged 2 commits into from
Nov 17, 2022
Merged

feat(rome_js_parser): export default function in declare module #3485 #3775

merged 2 commits into from
Nov 17, 2022

Conversation

denbezrukov
Copy link
Contributor

Summary

Fix #3485

  • Add new TsDeclareFunctionExportDefaultDeclaration in JsAnyExportDefaultDeclaration union.
  • Add TS_DECLARE_FUNCTION_EXPORT_DEFAULT_DECLARATION kind

Test Plan

cargo test

@denbezrukov denbezrukov requested a review from a team November 17, 2022 07:32
@netlify
Copy link

netlify bot commented Nov 17, 2022 •
edited
Loading

✅ Deploy Preview for docs-rometools canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 6f7ec45
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/637645bbe25cc900082e22d2

@denbezrukov denbezrukov force-pushed the export_default_declare branch 2 times, most recently from 009c5b7 to c5238db Compare November 17, 2022 07:58
MichaReiser
MichaReiser reviewed Nov 17, 2022
View reviewed changes
Copy link
Contributor

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Overall looking good. I recommend adding an optional id to the new introduced node and removing JsFunctionExportDefaultDeclaration from JsAnyExportDefaultDeclaration so that it is symmetric to regular function/class declarations

crates/rome_js_formatter/src/generated.rs Show resolved Hide resolved
crates/rome_js_formatter/src/ts/declarations/declare_function_export_default_declaration.rs Outdated Show resolved Hide resolved
crates/rome_js_parser/src/syntax/function.rs Outdated Show resolved Hide resolved
crates/rome_js_parser/src/syntax/function.rs Outdated Show resolved Hide resolved
xtask/codegen/js.ungram Show resolved Hide resolved
@denbezrukov denbezrukov force-pushed the export_default_declare branch from c5238db to 9a6bbd4 Compare November 17, 2022 12:21
@denbezrukov denbezrukov force-pushed the export_default_declare branch from 9a6bbd4 to 6a59be4 Compare November 17, 2022 12:25
@denbezrukov
Copy link
Contributor Author

@MichaReiser Could you please re-review the last commit?
6f7ec45

@MichaReiser MichaReiser merged commit 46f59a6 into rome:main Nov 17, 2022
@MichaReiser
Copy link
Contributor

Thanks for yet another awesome contribution!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

@xunilrj xunilrj Awaiting requested review from xunilrj

@ematipico ematipico Awaiting requested review from ematipico

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

🐛 Parser: export default function in declare module
2 participants
@denbezrukov @MichaReiser