prettier
Require a prettier config when requireConfig is true
Fix for #2402. When requireConfig is true we cannot rely on the resolvedConfig value to indicate that a Prettier config was found: the resolved configuration may have been picked up from an .editorconfig file instead.
This issue was previously fixed in 8.1.0 but the fix seems to have been undone by subsequent changes.
- [x] Run tests
- [x] Update the
CHANGELOG.mdwith a summary of your changes
I don't think this is right. I need to test, but I think configPath is just the location but doesn't necessarily indicate that there is a config at that location.
configPath is the resolution of prettier.resolveConfigFile given the current document path. That's null if no Prettier config file is found in the parent tree, coalesced to undefined in our code.
One thing to watch out for as you test: if you have a prettier config in your home dir or elsewhere in the parent tree then it'll get picked up by resolveConfigFile. That seems like the right behavior for this feature, but it's a potential red herring for testing.
This pull request has been labeled as stale due to inactivity. Reply to keep this pull request open.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
This pull request has been labeled as stale due to inactivity. Reply to keep this pull request open.
Keep open for next major release.
Actually, now that i think about this. Why can't you just set the config prettier.useEditorConfig to false? This will prevent the use of editorconfig as the config file and thus will require only a prettier config to be present.
@orenklein gave a good example in #2402: they're working in a monorepo with a global .editorconfig. Some of the individual packages in the monorepo augment those rules with Prettier configs, while others rely entirely on EditorConfig.
My team has a similar use case: we have a boilerplate .editorconfig in every repo to set broad formatting rules, and .prettierrc as an additional layer of configuration for projects that use Prettier.
In both use cases, it's helpful that Prettier allows us to extend EditorConfig rules and establish a hierarchy with a single source of truth for configurations that apply across multiple globs.
A couple of other factors I can think of:
- I work with projects from multiple teams, and it's useful to know that prettier-vscode will follow another team's Prettier config as best it can – including
editorconfig: true– without requiring me to make additional configurations specific to that project. - It's caught a few of us by surprise that EditorConfig satisfies the
requireConfigrule. That doesn't necessarily make it a bug, but I think the natural expectation is that prettier-vscode requires a Prettier config, not necessarily an EditorConfig.
I think your points are valid, but from the history of this debate I think some people are going to want the current behavior. I don’t love extra complexity, but I wonder is we should evolve this config into maybe three values.
useEditorConfig: “never” | “fallback” | “always”
I am leaning toward not using editor config as a default with the require config, but i feel like we should allow for it.
the next challenge will be picking the names of these config in a way that makes sense. Open to suggestions.
A suggestion for config naming: we could change the values for requireConfig instead of useEditorConfig. Something like this:
"requireConfig": "strict" | "inclusive" | "none"
-
strict= Files are formatted only if a Prettier configuration file is present -
inclusive= Files are formatted if Prettier or EditorConfig configuration files are present -
none= Formatting is always enabled (default)
We could then leave useEditorConfig as is and let people opt in to the new behavior explicitly while continuing to support people who've come to expect the current behavior.
This pull request has been labeled as stale due to inactivity. Reply to keep this pull request open.
This pull request has been labeled as stale due to inactivity. Reply to keep this pull request open.
This pull request has been labeled as stale due to inactivity. Reply to keep this pull request open.
This pull request has been labeled as stale due to inactivity. Reply to keep this pull request open.
@ntotten What's the next step here? Happy to try implementing either of the proposed config changes in this PR, but not sure which you'd prefer.
This pull request has been labeled as stale due to inactivity. Reply to keep this pull request open.