Skip to content
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

Add prettier to pretty the codes #3033

Closed
pdehaan opened this issue May 11, 2023 · 4 comments
Closed

Add prettier to pretty the codes #3033

pdehaan opened this issue May 11, 2023 · 4 comments

Comments

@pdehaan
Copy link
Contributor

pdehaan commented May 11, 2023

@Vinnl is right. Not using Prettier...YET. 😅

Originally posted by @maxxcrawford in #3030 (comment)

@pdehaan
Copy link
Contributor Author

pdehaan commented May 11, 2023

I gots curious, and it required a couple hoop jumps, so here are my early findings.

prettier.config.cjs

/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

module.exports = {
  trailingComma: 'none', // default is 'es5'
  tabWidth: 2,
  semi: false,
  singleQuote: true
}

package.json

git diff package.json | cat
diff --git a/package.json b/package.json
index feea063d6..53c4f8d4c 100644
--- a/package.json
+++ b/package.json
@@ -93,6 +93,7 @@
     "eslint-plugin-jsdoc": "^40.0.0",
     "node-mocks-http": "^1.12.1",
     "nodemon": "^2.0.20",
+    "prettier": "^2.8.8",
     "redis-mock": "^0.56.3",
     "stylelint": "^15.6.0",
     "stylelint-config-standard": "^33.0.0",

OUTPUT

npx prettier src/**/*.css --write

src/client/css/fonts.css 30ms
src/client/css/global.css 22ms
src/client/css/index.css 3ms
src/client/css/nav.css 7ms
src/client/css/partials/addEmail.css 5ms
src/client/css/partials/allBreaches.css 8ms
src/client/css/partials/breachDetail.css 13ms
src/client/css/partials/breaches.css 17ms
src/client/css/partials/emailPreview.css 1ms
src/client/css/partials/error.css 2ms
src/client/css/partials/exposureScan.css 10ms
src/client/css/partials/landing.css 11ms
src/client/css/partials/notFound.css 1ms
src/client/css/partials/settings.css 6ms
src/client/css/userMenu.css 4ms
src/client/css/variables.css 4ms
git status src

On branch main
Your branch is up to date with 'origin/main'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   src/client/css/fonts.css
	modified:   src/client/css/global.css
	modified:   src/client/css/nav.css
	modified:   src/client/css/partials/allBreaches.css
	modified:   src/client/css/partials/breaches.css
	modified:   src/client/css/partials/landing.css
git rev-parse HEAD # 2e761315a959e5bbf48dd32cea6a8fd28e140ad2
git rev-parse --abbrev-ref HEAD # main

@pdehaan
Copy link
Contributor Author

pdehaan commented May 11, 2023

OK, if I run this against the .js files, it gets a bit messier.

UPDATE: Filed ESLint+JSDoc config issue separately as #3035.


npm run lint:js
…
✖ 133 problems (0 errors, 133 warnings)
  0 errors and 1 warning potentially fixable with the `--fix` option.

133 warnings is... loud. And that's the current output from "main", not including any changes to Prettier.

npx prettier src/**/*.js --write

npm run lint:js
…
✖ 492 problems (359 errors, 133 warnings)
  359 errors and 1 warning potentially fixable with the `--fix` option.

Same warning count, but now we have 359 errors (although to be fair, 348 (97%) were "space-before-function-paren" errors).

npm run lint:js | grep -E "^\s+\d+:\d+\s+error\s+" | grep -E "space-before-function-paren$" | wc -l # 348
npm run lint:js | grep -E "^\s+\d+:\d+\s+error\s+" | grep -Ev "space-before-function-paren$" | wc -l # 11
npm run lint:js | grep -E "^\s+\d+:\d+\s+error\s+" | grep -Ev "space-before-function-paren$"

   44:7   error  Expected { after 'if' condition            curly
  32:15  error  Expected { after 'if' condition  curly
  188:1   error    Expected indentation of 8 spaces but found 10  indent
  189:1   error    Expected indentation of 8 spaces but found 10  indent
  190:1   error    Expected indentation of 6 spaces but found 8   indent
  192:1   error    Expected indentation of 8 spaces but found 10  indent
  193:1   error    Expected indentation of 8 spaces but found 10  indent
  194:1   error    Expected indentation of 6 spaces but found 8   indent
  27:1  error    Expected indentation of 2 spaces but found 10    indent
  37:1  error    Expected indentation of 2 spaces but found 14    indent
  44:1  error    Expected indentation of 2 spaces but found 14    indent

If When we're ready to adopt Prettier, I'd probably suggest we ignore the space-before-function-paren ESLint rule, and possibly indent (since both would be formatting rules managed by Prettier), and fix those two curly errors.

Other suggestions are probably to create a .prettierignore file and ignore the src/db/migrations/** folder and possibly others. Maybe initially ignore the src/e2e/** folder and tackle those files separately.

@Vinnl
Copy link
Collaborator

Vinnl commented May 12, 2023

If we were to adopt Prettier, we'd have to let go of "Standard" JS (which I refuse to ever write without quotes :P). I'd suggest removing all linting rules that are not about catching bugs, and just have Prettier to its auto-formatting - that's what we do on Relay as well. But I have an intense dislike for manual formatting.

@pdehaan
Copy link
Contributor Author

pdehaan commented Jun 20, 2023

Closing this in favor of @maxxcrawford's #3055

@pdehaan pdehaan closed this as completed Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants