-
-
Notifications
You must be signed in to change notification settings - Fork 447
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
Require a prettier config when requireConfig is true #2708
base: main
Are you sure you want to change the base?
Conversation
This commit fixes prettier#2402 by reintroducing changes from 5624041.
I don't think this is right. I need to test, but I think |
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. |
I'll merge this for v10 with prettier 3 as its a breaking change |
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 |
@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 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.
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:
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. |
Keep open. |
This pull request has been labeled as stale due to inactivity. Reply to keep this pull request open. |
Keep open. |
This pull request has been labeled as stale due to inactivity. Reply to keep this pull request open. |
Keep 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. |
Keep open? |
Fix for #2402. When
requireConfig
is true we cannot rely on theresolvedConfig
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.
CHANGELOG.md
with a summary of your changes