Remove prettier prod dependency in favor of user-installed formatter - #1639
Remove prettier prod dependency in favor of user-installed formatter#1639bluwy wants to merge 3 commits into
Conversation
Sorry, something went wrong.
🦋 Changeset detectedLatest commit: 237e280 The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Sorry, something went wrong.
|
|
||
| const prettierInstance = | ||
| options?.prettier !== false ? getPrettierInstance(cwd) : undefined; | ||
| const formatter = await getFormatter(options?.format ?? "auto", cwd); |
There was a problem hiding this comment.
how many of the supported formatters support formatting .md files?
Sorry, something went wrong.
There was a problem hiding this comment.
Ah good catch. Biome actually doesn't support it. The rest (prettier, deno, dprint) do. When detecting the formatter, should we prefer prettier in this case if both biome and prettier exist?
formatly currently detects and choose biome first, so we might need to workaround or fix that upstream 🤔
Another thought, if biome supports markdown in the future, do we unblock detecting biome within a semver minor/patch? Feels a bit of a gray area. EDIT: I think in practice it's ok in a minor, since if it affects anything it's only the dev workflow.
Sorry, something went wrong.
| if (!formatter) return async () => {}; | ||
|
|
||
| return async (filePath: string) => { | ||
| await formatly([filePath], { cwd, formatter }); |
There was a problem hiding this comment.
One caveat I notice is that since it calls a child process, it can take around 300-800ms after confirming the changeset in the CLI.
Filed: JoshuaKGoldberg/formatly#114
Sorry, something went wrong.
There was a problem hiding this comment.
JoshuaKGoldberg/formatly#144 should fix the performance issue. Do you have appetite to try that out for this PR?
I'm hesitant to merge the PR until we can confirm it works here.
Sorry, something went wrong.
There was a problem hiding this comment.
YOLO, merged it 🚀 . Available as formatly@0.3.0.
Sorry, something went wrong.
Codecov Report❌ Patch coverage is 90.00000% with 5 lines in your changes missing coverage. Please review.
@@ Coverage Diff @@
## next #1639 +/- ##
==========================================
- Coverage 72.25% 72.19% -0.06%
==========================================
Files 65 65
Lines 5013 4999 -14
Branches 874 874
==========================================
- Hits 3622 3609 -13
+ Misses 1386 1385 -1
Partials 5 5 ☔ View full report in Codecov by Sentry.
|
Sorry, something went wrong.
|
I'm going to close this as it's probably easier to start over |
Sorry, something went wrong.
|
but using formatly is probably the way to do it at least, it seems like a very nice solution |
Sorry, something went wrong.
Uses https://github.com/JoshuaKGoldberg/formatly to auto-detect the project's formatter for formatting the changesets and changelogs. Introduces a new format: "auto" | "prettier" | "biome" | "deno" | "dprint" | false option to control the formatting, and removes the prettier: boolean option in favor of this new option.
Tests has also been updated. Because test fixtures are inside /tmp directories with no access to prettier, the test assertions are now all against un-formatted code and has been updated accordingly.
NOTES: