Prettier-atom: Use prettier-eslint defaults when installed in project

[ryanhiebert]

As promised, making my case from my assertion in #216. If the concept is approved, and nobody is too antsy to wait for me to get around to implementing it, I'm willing to put in the work to make it happen.

---

Right now, using prettier-eslint and eslintConfig is the only way to configure what prettier-atom will do for a project, so it applies to all users of that codebase. Prettier unfortunately loses most of its value if it's not being used consistently across all contributors to a project, unless it's not being enforced by the project at all.

To that end, it's my belief that when the eslint integration is enabled, and is actually installed in the project, the only consistent thing to do would be to use the fallback values defined by prettier-eslint. I think that even the eslint integration would ideally be enabled automatically in prettier-atom if it's being used by project (noticed by pretter-eslint or prettier-eslint-cli being installed).

Looking around, I see that there are issues open regarding adding a prettier config file. When that happens, that of course would be a better setting to use as fallbacks, but I'm sure that prettier-eslint will use that as it's fallback instead, meaning that the fallback would _still_ not be implemented by prettier-atom.

That leaves the settings, ideally, as only being used when there is _no_ other configuration, expressed or implied, given by the project. Anything else makes it difficult to use a shared style through prettier.

---

In early issues on this front from the prettier repo, the thought was that the editor should reflect the _user's_ preferences, while the saved files should be able to enforce the _project's_ preferences. That's a very interesting idea, but if that's not done, it would be far better to use the project's preferences in the editor, than to default in whole, or especially in part, to the user's preferences.

[robwise]

I think that what probably should happen is that the configured settings not be used as the fallback configuration

This is something that we weren't originally doing and then a lot of people requested it so I took a lot of time to make this work (including changing prettier-eslint to accept fallback options). I think if people are using the eslint integration and they want everyone on the project to use semicolons, for example, they should add that as a rule to their eslint config. If that rule is missing from the eslint config, then that implies that the project doesn't care whether people use semicolons or not, so we fall back to the person's individual preferences. I don't see the use case where a project is using eslint, wants everyone to always use semicolons, but then doesn't want to add a semicolons rule to the eslint config?

[ryanhiebert]

First, thank you for all the hard work that you've put into prettier-atom. It has made my life less mired with dealing with styling, since I know that a quick save will fix things for me, and I can just _move on_ with my life.

---

If prettier is being used by a project, with or without eslint, its basic utility for the project is compromised if it doesn't format consistently between users. For me as a user, it also makes using prettier-atom difficult, and though I'm _willing_ to put up with difficulty because I like prettier and the integration from prettier-atom, I don't think it should be required, so I'm trying to make my case here.

I don't see the use case where a project is using eslint, wants everyone to always use semicolons, but then doesn't want to add a semicolons rule to the eslint config?

If all a project has is an eslintConfig, and they aren't using prettier, then you are correct. But if they are using prettier-eslint, then the expectation set by prettier is that the code style is, by even partial configuration _completely decided_, even though all parts are not mentioned explicitly.

In the example you gave above, the project _doesn't_ care whether or not semicolons are used, but if they are using prettier via prettier-eslint, they _do_ want it to be _consistent_. Consistent by means of the preferences of the tool, rather than their own, since they don't care, but inconsistency in formatting is _not_ what they want from prettier. They don't want two people to run prettier, and get different formatting.

What you've implemented in the way of allowing the user's preferences as a default is still valuable, but it seems to me that it should only be applied if the project itself is not using prettier. If it is, then the implicit contract with prettier is cross-user consistency, not merely the specified styles from eslintConfig being adhered to automatically.

---

So my position is still that, if the package.json is using prettier-eslint, then the fallback defaults should not be the editor's configuration, but the prettier-eslint defaults. If prettier-eslint is not in package.json, then I think it's perfectly reasonable, and quite nice, that prettier-atom would use its defaults wherever things are not specified in the eslintConfig (thank you again for the work you put into making it this way).

[robwise]

I agree that inconsistency between users on the same project is not desirable, I guess I was explaining in sort of a weird way. I'm saying that the intention of prettier-eslint is that you're supposed to eliminate this inconsistency by putting the rule in the eslint config and not rely on any fallbacks. That will solve this issue entirely, with the nice side-effect that your eslint script will pick up any cases where someone is not using prettier-eslint or not using it properly?

I was talking about the not caring whether semicolons are applied thing because that's the use case for fallbacks. We assume if you do not put the rule in your eslint config, then you are leaving it up to personal preference of each dev. If you want it to be consistent, it should go in the eslint config.

This behavior lets everybody win: you get to have consistency by placing the rules in the eslint config, and users who work on projects without eslint configs can still use the prettier settings that they prefer and not have to disable the eslint integration everytime they switch.

[ryanhiebert]

I agree that we should prefer the user's settings of prettier-atom when eslint doesn't give any guidance, as long as the project isn't actually using prettier. However, I can't think of a case where a project is using prettier, but wants the user to format it however the user desires. The project may not care about what the specific style is, but the contract of uniformity is still implied.

Let me take this out of the realm of eslint, and into the realm of a hypothetical prettier configuration. Let's say that prettier gets a configuration file, and there are two different projects. One sets the semi setting to false. In this case, it's clear that they don't want semicolons. If they instead set semi to true, then they would clearly _want_ semicolons.

But what if they simply didn't have any prettier configuration of the semi rule? Would it be correct to fall back to the user's preference, or is the default set by prettier an implied choice? I think that it must set using the default in that case, otherwise prettier's promise of consistency to the project is broken, as is the promise of taking the question out of formatting.

If the project isn't using prettier, then prettier-atom is purely a user choice, and we should prefer the user's preference. If prettier is used in a project, then prettier is _not_ just a user choice, and we need to respect the choice of the project, not make the _user_ jump through hoops to follow the implicit style of the project.

---

I feel like we're just arguing without getting anywhere, so unless you ask me for further comment, or I feel like you don't understand what I've said, this will be the last I say of it. I don't wish to keep wasting your time listening to me whine, even if I do think I'm right ;-)

[ryanhiebert]

Also, I'll note that for my own project I have gone ahead and added the rule that I needed to the eslint config, so that the question isn't an actual concern for me. I'm arguing my case on principle, because I think that there's a clear case that it should be the preferred choice. I see adding the eslint rule as a workaround, rather than fixing the problem, so I'm trying to help make things easier for everyone. I really don't see a downside to my suggestion.

[robwise]

We're not really arguing, just having a friendly discussion expressing our viewpoints on how to make the plugin better, which I always welcome :)

I guess it comes down to what is preferable if the project fails to properly configure their rules (whether it's an eslint config or a hypothetical prettier config if/when that's supported). With your way, it will help projects who've made this mistake have consistent styling anyway, with my way, people don't have to keep switching their eslint integration on and off every time they switch from a project that uses eslint to one that does not.

I guess one other option is add an option to toggle the fallback off?

[ryanhiebert]

We're not really arguing, just having a friendly discussion expressing our viewpoints on how to make the plugin better, which I always welcome :)

Good. I hope I don't ever make it seem otherwise. If I dip into unfriendly discussion, then that's the farthest thing from what I want to be doing.

I guess it comes down to what is preferable if the project fails to properly configure their rules

We disagree as to whether a partially-specified configuration is an improper configuration. I don't think that it is an improper configuration.

In prettier-atom, it's not possible to have partial configuration of prettier, since every setting has defaults, and the configuration cannot be disabled. Likewise, in both prettier and prettier-eslint, it's also not possible to have partial configuration, by way of well-known defaults. Unless we think that prettier would complain about a future configuration file not existing (which would be a breaking change), unspecified cannot mean unconfigured.

I guess one other option is add an option to toggle the fallback off?

A toggle to introduce this behavior would be acceptable to me, though I think not ideal or necessary. However, the toggle we would want wouldn't merely turn the fallback off, because then we'd have the same problem for users where they would have to choose between following the implied project settings when prettier was being used, or following their own preferences when they have the freedom to do so.

Instead, it should make it possible for prettier-atom to determine whether or not to use its own settings as the fallback based on the presence of prettier (in this case, really prettier-eslint) in the package.json.

A potential wording for such a setting might be:

• [ ] Use prettier (or prettier-eslint) defaults when prettier is installed in project
  _When prettier (or prettier-eslint) is installed in the project's dependencies, don't fall back to this editor configuration for prettier, but use the defaults from prettier (or prettier-eslint) instead._

  _Note: Since prettier doesn't currently have any configuration file, this currently only applies if prettier-eslint is installed._

[robwise]

We disagree as to whether a partially-specified configuration is an improper configuration. I don't think that it is an improper configuration.

Yeah, I guess you're right, this is what I'm getting hung up on. Why if you want to enforce the use of semicolons project-wide are you not putting that in the project's config?

[ryanhiebert]

Because I don't _care_. I just want to do whatever prettier does, and never consider it. That's why I'm using prettier, so that I don't have to think about all the possible stylistic preferences that prettier deals with.

If I don't agree with prettier's choices, it does _allow_ me to care about things, if I want to care about it. But then I don't have to care about everything, only the stuff that I care to change about prettier.

Sometimes I may _want_ to care about a particular preference, usually because it's different from prettier's default, and then the configuration is helpful. Otherwise, I don't want to care about it at all.

[robwise]

If you want to use the prettier defaults because you don't care, why did you change your prettier-atom settings to be non-default?

[ryanhiebert]

Because I do care as a _user_, but not as a project maintainer. When I'm doing things for my own way, then I'll use my preferences, but I'll be interacting with different projects that don't care, but still want things consistent. It's not just about me, of course, and sometimes when I use "I" I'm referring to different roles I may take. In answering your question, I was taking the role of the _project maintainer_, whereas in choosing my defaults in my editor I'm acting as a user.

As a user, my first priority is to follow the guidelines as set by the project I'm writing for. My second priority, if it's not incompatible, is to follow my own preferences. atom-prettier's configuration, by definition, is the user's preferences, not the project's.

As a user, I want the project preferences to take priority over my own, but if as a user I don't have the ability to change the project's prettier-eslint configuration (bureaucratic hassles or whatever), then I am stuck manually changing prettier-atom's settings when I switch projects, so that I can be sure that I'm following what I know is the implicit style indicated by the fact that the project is using prettier at all.

[vitorbal]

Now that prettier supports taking in a config file as of https://github.com/prettier/prettier/pull/2434, this discussion seems even more relevant.

[ryanhiebert]

Looks like this discussion was alluded to in that issue. The answer given to his question, though, was unsatisfying. Basically not making it clear what partial configuration means to the prettier team, unfortunately.

[zimme]

First, I haven't read through the entire thread.

I'll look into this as soon as I'm able. I've seen some unexpected behaviour when it comes to config and "load" order of configs in prettier-eslint.

I need to map out all cases of rule override and see if there maybe needs to be some extra option to support all cases when using prettier-eslint with an editor. Right now I believe that it works as expected using the cli.

Options provided via cli overrides options in config files, at least for prettier. I do believe this is backwards when it comes to eslint.

Anyhow, I'll keep you updated once I've had time to look into this.

[robwise]

@zimme Note that on prettier-eslint I implemented fallbackOptions which are options that only get used if they weren't specified otherwise

[zimme]

@robwise Yes, I've seen that. I just need to look into how it is used. I'm just guessing here, but I'm thinking that you send in the projects config via prettierOptions and then send in the user configured options as the fallbackOptions?

[robwise]

We use fallbackOptions built from first any prettier config, then an editorconfig if it exists, then the user's atom config. Otherwise prettier-eslint wouldn't use the eslint config values.

https://github.com/prettier/prettier-atom/blob/master/src/executePrettier/buildPrettierEslintOptions.js#L9
https://github.com/prettier/prettier-atom/blob/master/src/executePrettier/buildPrettierOptions.js#L66-L68

[zimme]

@robwise I've been thinking about this a bit and the only way I see that we can "resolve" this unexpected but "correct" behaviour is if I remove the fallbackPrettierOptions and export a function getPrettierConfig(eslintConfig?) that will give you a prettier config inferred using the eslintConfig you provide or if none is given, the config eslint will give us based on the filePath/working directory.

Then you should be able to decide which prettier config to used in this order.

1. Use prettiers resolveConfig and use that if you got one.
2. Use prettier-eslints getPrettierConfig(eslintConfig?) and use this config if you got one.
3. Use prettier-atoms user provided config.

For this to work I will have to change the way prettier-eslint handles the case when the consumer of the api provides prettierOptions/eslintConfig.

Does this make sense?

[robwise]

I get what you're saying, but I feel like it would complicate things for not much gain? Like, if you want to use prettier-eslint and have it format a certain way, configure it to format that way via your eslint config, if you constantly create new projects and don't want to have to specify a config every time, set your fallback options in your prettier-atom settings. Either way, I would think you'd be covered?

[ryanhiebert]

What that means to me, as someone who wants to use prettier-atom all the time, so that I can keep from having to think about these formatting issues, is that if I find myself wishing to contribute to a project that 1) uses prettier and 2) it's not appropriate or convenient for me to change the configuration, and 3) doesn't configure every possible setting, but rather relies on the defaults, that I'll find myself with these options:

1. Turn off prettier-atom, and just use whatever method the project has for formatting using the prettier CLI. No format on save, which is one of the reasons that I've come to love using prettier-atom.
2. Switch my personal settings in prettier-atom to match a project that I want to contribute to, but really don't care about their formatting choices. Then probably have to change it back, because that doesn't change my personal preferences, it only means that I have to tell prettier-atom somebody else's preferences as if they are my own.
3. Switch the project's prettier settings while I'm working on it, to declare all the setting explicitly to make things happy, then undo that because it's not a part of the pull request I'm trying to construct on the project.

I want the option to have prettier notice that there are defaults already expected by the project, and that they should be respected. I don't want prettier-atom to make me do a song and dance to keep using prettier-atom, I just want to make a little contribution without making me or anyone else jump through hoops.

You are free to decide that's not worth it to you to make this change to this project, but then you're saying that one of those three options is better than the change I'm suggesting, perhaps because you think that every project that will specify configuration will, or should, specify every setting, rendering the problem scenario to be an edge-case that's not worth considering. Obviously I disagree, but that's fine because it's your project, not mine. Just please understand that is what you're deciding.

---

Because this always does seem like an adversarial debate, I want to reiterate that I really have appreciated your work on prettier-atom, and that I respect the contributions that you've given to the community and to me personally, despite wherever we may end up as a decision on this topic.

Thank you for how you have made this corner of my programming world a nicer place to be.

[robwise]

It's not adversarial, don't worry about it! I just was having a hard time understanding the issue properly I think. I think now I get what you're saying:

• You have personal prettier-atom settings that you like to use for your own projects so that you don't have to keep making prettier configs in your own projects
• You contribute to other projects that use prettier but have only partially defined options in their prettier config and are relying on prettier to use the default values for the undefined options

So when you save in one of these other projects, your incorrect personal prettier-atom settings are used instead of the defaults that the project was relying on?

So, therefore, the proposal is to have an option that, when enabled, notices that if _any_ prettier options are configured, that it should use prettier defaults and ignore all personal prettier-atom settings. Only if _no_ prettier options are configured should it use the personal prettier-atom settings?

[ryanhiebert]

That's right. Or more accurately, the prettier config isn't necessary either. I think it should be keyed on whether or not it's in the package.json, since the project may be perfectly happy with the all of the defaults of prettier (or prettier-eslint).

[ryanhiebert]

You have personal prettier-atom settings that you like to use for your own projects so that you don't have to keep making prettier configs in your own projects

And for JS files that aren't really part of a project with a package.json, but I still have formatting control. For instance, if I'm writing a bit of javascript that's just one file or a simple script or something.

[ryanhiebert]

So, therefore, the proposal is to have an option that, when enabled,

And sorry for the flood, but I'm also suggesting that this would be a reasonable default as well. A setting would be fine, just set it once and forget about, but I think this is perfectly reasonable behavior to expect across the board.

[zimme]

So the problem I can see here if we are going to try and detect this automatically is when a project is using prettier and their "config" is to use prettiers defaults. i.e. no config.

We can't really detect this and fall back to using the prettier-atom config, right?

This should be doable in the context of prettier-eslint and not just prettier.

[ryanhiebert]

Why not? We already do that for the prettier-eslint integration, don't we? When the box is checked we automatically use prettier-eslint _if_ it's installed in the project? Whether that's looking for the package in node_modules or looking at the package.json to see if it's listed, I'm not sure, but that same mechanism seems like an appropriate way to check for prettier in general.

[robwise]

Yeah, what I'm envisioning code-wise to get this done would be a change that would work regardless of whether the user is using prettier-eslint or not. Basically, if we detect that prettier is in the package.json and this option is enabled, we just don't pass any options from prettier-atom. That's really all, it would be a pretty simple change I think.

[olsonpm]

Or more accurately, the prettier config isn't necessary either. I think it should be keyed on whether or not it's in the package.json, since the project may be perfectly happy with the all of the defaults of prettier

I just want to clarify one difference in the implementation that we've landed on over what OP expected above.

Since it's reasonable for a project to list prettier as a dependency without actually wanting it to format their codebase, I think a configuration is a non-ambiguous heuristic for "intent to format". If a project wants all defaults they should list an empty config.

This is the condition #374 uses when deciding whether to pass any prettier-atom settings onto prettier.