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

Hug object types in function parameters in Flow like in TS #13396

Merged
merged 5 commits into from Aug 30, 2022

Conversation

thorn0
Copy link
Member

@thorn0 thorn0 commented Aug 28, 2022

Description

Fixes #13395

Checklist

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.

Try the playground for this PR

@thorn0 thorn0 linked an issue Aug 28, 2022 that may be closed by this pull request
@thorn0

This comment was marked as resolved.

@thorn0 thorn0 marked this pull request as ready for review August 29, 2022 06:57
(node, name) =>
node.type === "FunctionTypeParam" && name === "typeAnnotation",
shouldHugParameter
))) ||
Copy link
Member Author

Choose a reason for hiding this comment

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

Would be cool if path.match supported alternatives like this:

path.match(
  isObjectType,
  [
    [
      (node, name) => name === "typeAnnotation",
      (node, name) => name === "typeAnnotation"
    ],
    [
      (node, name) =>
        node.type === "FunctionTypeParam" && name === "typeAnnotation"
    ]
  ],
  shouldHugParameter
)

((number === 0 && (name === "params" || name === "parameters")) ||
name === "this" ||
name === "rest") &&
shouldHugFunctionParameters(node)
Copy link
Member

@fisker fisker Aug 30, 2022

Choose a reason for hiding this comment

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

If function has .this and normal parameters, won't this pass on second parameter?

Copy link
Member

Choose a reason for hiding this comment

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

Use getFunctionParameters().indexOf?

Copy link
Member Author

Choose a reason for hiding this comment

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

shouldHugFunctionParameters checks that there is only one parameter, but yes, doing what you propose would be easier to follow.

Copy link
Member

Choose a reason for hiding this comment

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

If shouldHugFunctionParameters only pass on one parameter. The number check is redundant. Only need make sure it's a parameter.

Copy link
Member

Choose a reason for hiding this comment

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

I'm a little bit confused, the "node" is a function, right? It can't be another function's parameter, how name can be "parameters"? The "name" is key of child?

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes, this is different from callback of AstPath#each, the name is key of node there.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, getFunctionParameters().indexOf looks clumsy because I need to pass a node to it, and to get this node I have to use name and number anyway. So I'm going to simply remove the number check.

Copy link
Member Author

Choose a reason for hiding this comment

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

I also renamed shouldHugFunctionParameters to shouldHugTheOnlyFunctionParameter to make it clear that it checks that there is only one parameter.

@@ -157,7 +157,7 @@ function printFunctionParameters(
];
}

function shouldHugFunctionParameters(node) {
function shouldHugTheOnlyFunctionParameter(node) {
Copy link
Member

Choose a reason for hiding this comment

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

👍

@thorn0 thorn0 merged commit f5610b8 into prettier:next Aug 30, 2022
@thorn0 thorn0 deleted the fix-flow-declare-function branch August 30, 2022 14:38
medikoo pushed a commit to medikoo/prettier-elastic that referenced this pull request Jan 31, 2024
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 this pull request may close these issues.

declare function formatted differently for Flow and TypeScript
3 participants