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(js_formatter): Allow function expressions to group and break as call arguments #1003

Merged
merged 4 commits into from
Dec 2, 2023

Conversation

faultyserver
Copy link
Contributor

Summary

Addressing the first case of #914. Playground Link.

This is effectively an extension of #934, utilizing the new ParameterLayout::Compact so long as call_arguments decides that the expression can be grouped.

But, the previous logic only actually formatted as a grouped expression if the function was not the only call argument and had at least one parameter, which was too restrictive. The logic now more closely matches Prettier's logic, using the compact format when the function is not the only call argument or has only simple parameters.

The definition of a simple parameter is somewhat loose, and not an exact match to Prettier, but is close. Prettier says "an identifier with no type annotation", but the semantics of estree mean that parameters with an initializer are technically Assignment nodes, while in Biome they are just JsFormalParameters with .initializer().is_some().

Test Plan

Added test cases for the specific grouping behavior, and the prettier diff snapshot for typescript has been removed.

@github-actions github-actions bot added A-Formatter Area: formatter L-JavaScript Language: JavaScript and super languages labels Dec 1, 2023
Copy link

netlify bot commented Dec 1, 2023

Deploy Preview for rad-torte-839a59 failed.

Name Link
🔨 Latest commit 6c5e2ca
🔍 Latest deploy log https://app.netlify.com/sites/rad-torte-839a59/deploys/656a765edd6b6e00083e7491

Copy link
Member

@Conaclos Conaclos left a comment

Choose a reason for hiding this comment

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

Looks Good to me! I left some minor suggestions :) Feel free to fix and merge :)

@faultyserver
Copy link
Contributor Author

Refactored a bit to also catch cases for arrow expressions that i saw after the fact, but the logic is all the same, and i applied those suggestions when moving the code as well.

@faultyserver faultyserver merged commit eddf6d3 into main Dec 2, 2023
14 of 18 checks passed
@faultyserver faultyserver deleted the faulty/function-expression-breaking branch December 2, 2023 00:24
yossydev pushed a commit to yossydev/biome that referenced this pull request Dec 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Formatter Area: formatter L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants