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): fix function parameter grouping heuristic checking type parameters #1153

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

faultyserver
Copy link
Contributor

Summary

Playground Link.

This was a minor bug that only shows up two times in our entire monorepo: when a function declaration has a single generic type parameter, the parameters for that function sometimes end up getting grouped so that they prefer to break before the return type for the function does. If the parameter has a constraint (extends ...) or a default value (= DefaultType), then the parameters don't group, but if neither are present, then the parameters might group based on further heuristics.

However, the logic here had the constraint checked flipped from is_some() to is_none(), meaning the inverse was happening, as seen in the playground.

Test Plan

There was a prettier diff snapshot that I didn't even notice covering this case, but that's deleted now since the output matches.

Copy link

netlify bot commented Dec 12, 2023

Deploy Preview for biomejs ready!

Name Link
🔨 Latest commit b02c26a
🔍 Latest deploy log https://app.netlify.com/sites/biomejs/deploys/6578049efe3ce200088ad4e3
😎 Deploy Preview https://deploy-preview-1153--biomejs.netlify.app/playground
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

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

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Good catch!

@ematipico ematipico merged commit 474c956 into main Dec 12, 2023
18 checks passed
@ematipico ematipico deleted the faulty/ts-return-grouping branch December 12, 2023 08:27
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.

None yet

2 participants