Skip to content
This repository was archived by the owner on Mar 3, 2023. It is now read-only.

Add .eslintrc.json to support use of Prettier code formatter#19296

Merged
nathansobo merged 1 commit intomasterfrom
ns/eslintrc
May 9, 2019
Merged

Add .eslintrc.json to support use of Prettier code formatter#19296
nathansobo merged 1 commit intomasterfrom
ns/eslintrc

Conversation

@nathansobo
Copy link
Contributor

On other projects, I've used the Prettier code formatter via the official Atom package. It saves a lot of time hassling with code formatting, and not having it enabled for Atom is causing me to waste a lot of time fixing linting errors.

On Atom, we're using JS Standard Style, which conflicts with the Prettier's default formatting. This is okay, because the Prettier package can run eslint --fix automatically after performing its own formatting, but it requires a bit of configuration. To support it, I've added an .eslintrc.json file to the root of the module. This needs to pull in the shared ES Lint configurations associated with Standard Style, but since we avoid installing dev dependencies in our root package.json, I've added those linting configurations to script/package.json and am including them with an explicit path.

With this change, you can now get nice auto-formatting when you install Prettier Atom and enable ES Lint support in the package settings.

Screen Shot 2019-05-09 at 10 40 24 AM

We have a lot of files that aren't formatted with prettier, so this might generate a bit of churn initially, but I think the productivity savings will be worth it in the long run.

One particular issue is that Atom's source files don't have a consistent style with respect to spaces inside of curly brackets. Prettier defaults to having spaces ({ foo: "bar" }, but standard doesn't seem to care, and we have examples of both styles in this repo. It's possible to override the default and remove spaces, but it's more typical style to include these spaces, so I'm going to leave the current default in place.

cc @atom/maintainers

@Arcanemagus
Copy link
Contributor

I'd recommend adding a compatible version of eslint as a dependency as well so it is tracked with the plugins that use it.

@nathansobo
Copy link
Contributor Author

@Arcanemagus the plugin I'm using bundles its own version of eslint, and I'm reluctant to mess with the linting infrastructure right now. I'm going to merge this. If there's something I'm not fully understanding however about the benefits then I am open to a follow-on PR.

@nathansobo nathansobo merged commit e15f9fa into master May 9, 2019
@nathansobo nathansobo deleted the ns/eslintrc branch May 9, 2019 22:05
@rafeca
Copy link
Contributor

rafeca commented May 10, 2019

That's cool! I'm going to try it!

As a followup, it would be cool to add first-class support for prettier (so it can be executed across all codebase by running script/lint --fix or something similar) and introduce the eslint rule to fail if any file is not prettified.

This way we avoid files going back and forth in codestyle depending on whether the committer is using prettier

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants