Skip to content

fix(lines-before-block): Only trigger after ';', '}', '|', and '&' #1383

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 3 commits into from
May 10, 2025

Conversation

LukeAbby
Copy link
Contributor

@LukeAbby LukeAbby commented May 10, 2025

Fixes #1379 and #1343.

I initially went for a targeted fix that would only solve my issue, #1379, but I noticed that all punctuators didn't seem to me like reasonable places to insert a line before. As I had also been facing #1343 solving it was nice too.

Of course what "feels" right is obviously subjective so let me know if you want me to put this behind an option. I considered exposing lintedPunctuators as an option but it felt too granular. Disabling & could be useful to users of Prettier that want to set ignoreSameLine to false though. Specifically, Prettier really isn't fond of the formatting in this snippet:

type IntersectionDocumentation =
    & { someProp: number }
    /** Description. */
    & { otherProp: string };

And will format it like so:

type IntersectionDocumentation = { someProp: number } /** Description. */ & { otherProp: string };

Which I think looks terrible.

As a final note, currently the option checkBlockStarts is set to actually control linting after all punctuators because I noticed a number of tests expected it to work that way for more than just {, like the array test etc. This makes it a bit of a misnomer but it appears it was already a bit of a misnomer.

@LukeAbby LukeAbby force-pushed the lintOnlyWhitelistedPunctuators branch from 3588c61 to 20bdc67 Compare May 10, 2025 07:14
Copy link
Collaborator

@brettz9 brettz9 left a comment

Choose a reason for hiding this comment

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

This LGTM. Just the suggested tweak on using a Set.

@LukeAbby LukeAbby requested a review from brettz9 May 10, 2025 14:02
@LukeAbby
Copy link
Contributor Author

Thanks for the review! Bad habit of mine to not use a Set. Probably comes from knowing that Set#has is slower than Array#includes for small array but it leads to clearer code and that's what's most important.

@brettz9 brettz9 merged commit 19fa3dc into gajus:main May 10, 2025
5 checks passed
Copy link

🎉 This issue has been resolved in version 50.6.13 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lines-before-block should not trigger for the first item inside functions and constructors
2 participants