fix(language-server): preserve prettier config when resolveConfig returns null#16174
fix(language-server): preserve prettier config when resolveConfig returns null#16174spaciousejar wants to merge 1 commit into
Conversation
…urns null Fixes issue where trailing commas were incorrectly removed when prettierInstance.resolveConfig() returns null. The fix ensures that user's Prettier configuration is properly respected by initializing configOptions to an empty object and only overriding it when resolveConfig returns a valid configuration. Fixes withastro#14665
|
There was a problem hiding this comment.
Pull request overview
Fixes Astro language server formatting so user Prettier settings (notably trailingComma: "all") are still respected when prettier.resolveConfig() returns null, preventing formatting from unintentionally dropping trailing commas on save.
Changes:
- Initialize Prettier
configOptionsto{}and only replace it whenresolveConfig()returns a non-null config. - Add a null-check around the
resolveConfig()result to avoid clobbering downstream merged options.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const resolvedConfig = await prettierInstance.resolveConfig(filePath, { | ||
| // This seems to be broken since Prettier 3, and it'll always use its cumbersome cache. Hopefully it works one day. | ||
| useCache: false, | ||
| editorconfig: true, | ||
| }); | ||
| if (resolvedConfig) { | ||
| configOptions = resolvedConfig; |
There was a problem hiding this comment.
The identifier resolvedConfig is used for two different concepts (the result of prettierInstance.resolveConfig() inside the try, and then the merged config object later). Even though the scopes don’t overlap, this makes the config-resolution logic harder to follow; consider renaming the inner variable to something like fileConfig / resolvedFileConfig (or renaming the merged one) to avoid confusion during future edits.
| const resolvedConfig = await prettierInstance.resolveConfig(filePath, { | |
| // This seems to be broken since Prettier 3, and it'll always use its cumbersome cache. Hopefully it works one day. | |
| useCache: false, | |
| editorconfig: true, | |
| }); | |
| if (resolvedConfig) { | |
| configOptions = resolvedConfig; | |
| const fileConfig = await prettierInstance.resolveConfig(filePath, { | |
| // This seems to be broken since Prettier 3, and it'll always use its cumbersome cache. Hopefully it works one day. | |
| useCache: false, | |
| editorconfig: true, | |
| }); | |
| if (fileConfig) { | |
| configOptions = fileConfig; |
| const resolvedConfig = { | ||
| filepath: filePath, | ||
| tabWidth: formatOptions.tabSize, | ||
| useTabs: !formatOptions.insertSpaces, | ||
| ...editorOptions, | ||
| ...configOptions, | ||
| }; | ||
| } as any; |
There was a problem hiding this comment.
resolvedConfig is force-cast to any, which removes type-checking for downstream uses (e.g. resolvedConfig.plugins) and can hide accidental option-shape regressions. Consider typing editorOptions/configOptions as Record<string, unknown> or Partial<prettier.Options> (via a type import) and giving the merged object an explicit options type so you can avoid as any here.
| let configOptions = {}; | ||
| try { | ||
| configOptions = await prettierInstance.resolveConfig(filePath, { | ||
| const resolvedConfig = await prettierInstance.resolveConfig(filePath, { | ||
| // This seems to be broken since Prettier 3, and it'll always use its cumbersome cache. Hopefully it works one day. | ||
| useCache: false, | ||
| editorconfig: true, | ||
| }); | ||
| if (resolvedConfig) { | ||
| configOptions = resolvedConfig; | ||
| } |
There was a problem hiding this comment.
This change adjusts how Prettier config resolution behaves when resolveConfig() returns null, but there’s no regression test covering the reported trailing-comma scenario. Since the language server already has Prettier formatting tests (test/misc/prettier-format.test.ts), please add a test that formats an .astro document with a .prettierrc (e.g. trailingComma: "all") and asserts the formatted output preserves trailing commas, so this doesn’t regress again.
Summary
Fixes issue where trailing commas were incorrectly removed when `prettierInstance.resolveConfig()` returns null.
Problem
The Astro VSCode extension v2.8.1+ incorrectly removes trailing commas from multi-line function calls when saving `.astro` files, even when Prettier is configured to preserve them with `trailingComma: "all"`.
Root Cause
In `packages/language-tools/language-server/src/languageServerPlugin.ts`, when `prettierInstance.resolveConfig()` fails or returns `null`, the `configOptions` variable remains `null`. During the configuration cascade (`...configOptions`), this causes the user's Prettier configuration to be completely ignored.
Solution
Changes
Testing
Fixes #14665