Skip to content

Avoid extra offset in arrow function body when using long types (alternative solution) #11515

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

Merged
merged 9 commits into from
Nov 17, 2021

Conversation

thorn0
Copy link
Member

@thorn0 thorn0 commented Sep 13, 2021

Description

This is an alternative to #10956. It uses the following formatting:

const MyComponent2: React.VoidFunctionComponent<
  MyComponent2Props
> = ({ x, y }) => {
  const a = useA();
  return (
    <div>
      x = {x}; y = {y}; a = {a}
    </div>
  );
};

@kachkaev How does this look to you?

Regression testing (prettier/prettier-regression-testing#73 (comment)) has shown that "prefer to keep the arrow signature on one line if possible" is a bad idea in this case.

Testing results for the current version of this PR: prettier/prettier-regression-testing#73 (comment)

Closes #10848

Checklist

  • I’ve added tests to confirm my change works.
  • (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

Sorry, something went wrong.

@thorn0 thorn0 marked this pull request as draft September 13, 2021 01:59
Regression testing showed that this was a bad idea in this case
@thorn0 thorn0 marked this pull request as ready for review September 13, 2021 11:08
@kachkaev
Copy link
Member

👋 @thorn0! The diff in integration tests makes absolute sense to me, so I’m more than happy to proceed with your solution 😍

Copy link
Member

@sosukesuzuki sosukesuzuki left a comment

Choose a reason for hiding this comment

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

Overall looks good! But diffs are large, so I think we should release this as minor.

@kachkaev
Copy link
Member

kachkaev commented Sep 14, 2021

@sosukesuzuki this PR fixes a regression in Prettier 2.3.0 – the new behaviour closely matches 2.2.1. Given this and the fact that 2.4.0 is quite new, would it be reasonable to include the PR into 2.4.1?

If this was a completely new behaviour, I'd agree with you, but a former change between 2.2 and 2.3 tweaks the weights a bit I guess 🙂

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

I like it

@sosukesuzuki
Copy link
Member

Hmmm... I still think this should be released in 2.5, as it is not a fix for the regressions that occurred in 2.4, and it would create a large diff. Many users will not like a big diff in a patched version. (But I'm not confident that this is the best way).

@sosukesuzuki sosukesuzuki added this to the 2.5 milestone Sep 22, 2021
@sosukesuzuki sosukesuzuki merged commit 11e43e2 into prettier:main Nov 17, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 18, 2022
@thorn0 thorn0 deleted the alt-10848 branch December 28, 2022 13:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The offset for React component body depends on component name length in v2.3
4 participants