search

Turfjs / turf

A modular geospatial engine written in JavaScript and TypeScript
https://turfjs.org/
MIT License
9.25k stars 937 forks source link

Rework prettier setup #2677

Closed mfedderly closed 1 month ago

mfedderly commented 1 month ago

Since I was already here I started by upgrading husky prettier and lint-staged.

It turns out that we intentionally dialed down the prettier aggressiveness because the formatting of test fixtures looked pretty ugly with the existing prettier settings. I think that if we increase the target printWidth for prettier, we can more confidently re-enable it for all of the test fixtures.

I think that even imperfect formatting of the fixtures is better than inconsistent formatting because of disabling it.

Its probably easiest to review this commit by looking at the first one to understand the changes, and the second commit for getting a sense of how the changes impacted the formatting across the codebase.

smallsaucepan commented 1 month ago

Personally would rather see code stay where it is (80 right?). Changing that is going to lead to unnecessary churn in pretty much every file.

For geojson, as we can set a different width, how about we experiment repository wide with a couple of different ones? For example, if 90 results in the majority of our single line [1, 2, 3, 2, 1] arrays wrapping, try a larger value until it's just a handful.

mfedderly commented 1 month ago

@smallsaucepan this PR does exactly what you suggest, everything stays at 80 except .json and .geojson which are closer to 90 (printWidth is not a hard cap, just a suggestion). I think that 90 seems to strike a pretty nice balance for the geojson not wrapping things too aggressively.

mfedderly commented 1 month ago

Going to merge this now. If we have outstanding PRs with .geojson or .json changes we should make sure to merge in master to get a fresh build and make sure we don't merge a prettier break.