syunto07ka / jewelry_box

2 stars 0 forks source link

ts及びtsxファイルに対してコードフォーマットをかける #73

Closed syunto07ka closed 4 years ago

syunto07ka commented 4 years ago

close #52

syunto07ka commented 4 years ago

あ、

eslint-plugin-prettier exposes a "recommended" configuration that configures both eslint-plugin-prettier and eslint-config-prettier in a single step.

https://prettier.io/docs/en/integrating-with-linters.html#recommended-configuration

ここ修正しますね

=> 277818b にて

piro0919 commented 4 years ago

@syunto07ka

ちょっとちぐはぐな状況になっていますね。 🐘 一つずつ確認していきましょうか。

まずprettierを導入されたと思うのですが、prettier自体が使用されているように見えません。 そのためprettier及びprettierに関わるeslintの設定(eslint-config-prettier及びeslint-plugin-prettier)は不要だと思いますが、どうでしょうかー?

syunto07ka commented 4 years ago

@piro0919

まずprettierを導入されたと思うのですが、prettier自体が使用されているように見えません。

👀 本当ですか? eslintとprettierを併用する設定にできてないですかね?prettierの設定はデフォルトで値が設定されていると思ってて、現に eslint --fix でコードフォーマットができるようになったのですが。

それともこの設定自体がeslintでできるからprettierいらない、という意味でしょうか?

piro0919 commented 4 years ago

@syunto07ka

今回コードフォーマットに使用するスクリプトは eslint --fix だと思われますが、では逆にどこでprettierを使用されていますか?

ちなみに、コードフォーマットにeslintを使用している以上、prettierを導入する理由がないのでは?と思っています。

syunto07ka commented 4 years ago

@piro0919

All these instructions assume you have already installed prettier in your devDependencies.

prettierが入っている前提での設定だから、prettierの設定値を使ってeslintがコードフォーマッティングしてるわけではない。と思ってるのですが。。? 質問の答えとしては eslint --fix の中でprettierが走ってると思っています。 (ちゃんと見たわけじゃなくただの予想だが、eslintlintConfigで設定したplugin:prettier/recommended によって eslint --fix 時にprettierのコマンドが走るのではと考えている)

https://prettier.io/docs/en/integrating-with-linters.html

syunto07ka commented 4 years ago

ちゃんと効いてるっぽい

スクリーンショット 2020-05-16 20 09 30

piro0919 commented 4 years ago

@syunto07ka

ちょっと理解しきれていなくて申し訳ないです。 🙇

質問の答えとしては eslint --fix の中でprettierが走ってると思っています。

prettierをインストールしないと eslint --fix が動かない、という認識ですかね? もしくはeslint-config-prettierまたはeslint-plugin-prettierが動作しない、という認識ですかね?

もう一件、少し話を戻しまして。

そもそも、なぜeslintにprettierに関する設定を導入しましたか? そこの根拠はほしいですね。

syunto07ka commented 4 years ago

もしくはeslint-config-prettierまたはeslint-plugin-prettierが動作しない、という認識ですかね?

prettierがないことによって上記が動作しない。

prettierをインストールしないと eslint --fix が動かない、という認識ですかね?

したがってこちらも動作しないという認識です。

syunto07ka commented 4 years ago
 ~/development/jewelry_box  set_cord_formater●  0m  ./node_modules/.bin/eslint --fix src/containers/index.tsx                                                                                      
 ~/development/jewelry_box  set_cord_formater  0m 

成功

 ~/development/jewelry_box  set_cord_formater  0m  npm uninstall prettier                                                                                                                          
npm WARN eslint-plugin-prettier@3.1.3 requires a peer of prettier@>= 1.13.0 but none is installed. You must install peer dependencies yourself.

removed 3 packages and audited 2847 packages in 14.641s

135 packages are looking for funding
  run `npm fund` for details

found 6 vulnerabilities (5 low, 1 high)
  run `npm audit fix` to fix them, or `npm audit` for details
 ~/development/jewelry_box  set_cord_formater●  0m 
 ~/development/jewelry_box  set_cord_formater●  0m  ./node_modules/.bin/eslint --fix src/containers/index.tsx                                                                                      
Error: Cannot find module 'prettier'
Require stack:
- /Users/syunto/development/jewelry_box/node_modules/eslint-plugin-prettier/eslint-plugin-prettier.js
- /Users/syunto/development/jewelry_box/node_modules/eslint/lib/cli-engine/config-array-factory.js
- /Users/syunto/development/jewelry_box/node_modules/eslint/lib/cli-engine/cascading-config-array-factory.js
- /Users/syunto/development/jewelry_box/node_modules/eslint/lib/cli-engine/cli-engine.js
- /Users/syunto/development/jewelry_box/node_modules/eslint/lib/cli-engine/index.js
- /Users/syunto/development/jewelry_box/node_modules/eslint/lib/cli.js
- /Users/syunto/development/jewelry_box/node_modules/eslint/bin/eslint.js
Occurred while linting /Users/syunto/development/jewelry_box/src/containers/index.tsx:1
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:980:15)
    at Function.Module._load (internal/modules/cjs/loader.js:862:27)
    at Module.require (internal/modules/cjs/loader.js:1040:19)
    at require (/Users/syunto/development/jewelry_box/node_modules/v8-compile-cache/v8-compile-cache.js:161:20)
    at Program (/Users/syunto/development/jewelry_box/node_modules/eslint-plugin-prettier/eslint-plugin-prettier.js:163:26)
    at /Users/syunto/development/jewelry_box/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/Users/syunto/development/jewelry_box/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/Users/syunto/development/jewelry_box/node_modules/eslint/lib/linter/node-event-generator.js:254:26)
    at NodeEventGenerator.applySelectors (/Users/syunto/development/jewelry_box/node_modules/eslint/lib/linter/node-event-generator.js:283:22)
 ~/development/jewelry_box  set_cord_formater●  0m 
syunto07ka commented 4 years ago

@piro0919 実際にテストしてみましたが、prettierないとerror吐きますね

syunto07ka commented 4 years ago

@piro0919

そもそも、なぜeslintにprettierに関する設定を導入しましたか?そこの根拠はほしいですね。

こちらに関しては、実際にeslintでspaceのルールなど入れてみたときにText部分に関する効きが悪かったです。今回prettierを使ってみて問題なく動作したことを確認したので導入に至っています。

js側のエラーをESLintに、コード自体の書き方のエラーはPrettierに のような棲み分けも綺麗かなと思いましたし

piro0919 commented 4 years ago

@syunto07ka

実際にテストしてみましたが、prettierないとerror吐きますね

了解です素晴らしい。 👍

こちらに関しては、実際にeslintでspaceのルールなど入れてみたときにText部分に関する効きが悪かったです。今回prettierを使ってみて問題なく動作したことを確認したので導入に至っています。

結論から書くと、prettierを導入されているのにprettierのcliを使われていないのが個人的に違和感がありました。 ざっくりと調べてみたところ、フォーマッターのかけかたとしては以下の2種類があるみたいです。

  1. prettierのcliでかける → eslintのcliでかける
  2. eslintのcliでかける

調べた感じ2.で問題なさそうでした。 ちなみに、自分はよく1.の方法を使っていました。 👍

また、eslintでフォーマッタをかける場合に、prettierを選んだ根拠は欠けるのかなぁとはやはり思いますね。 eslint-config-airbnbなどをはじめ、様々な選択肢があると思いますので、その中でなぜprettierを選択したのかな?と思った次第です。

かなり複雑な部分なので、自分の知識が足りていないのがなんとも申し訳ないです。 🙇

syunto07ka commented 4 years ago

また、eslintでフォーマッタをかける場合に、prettierを選んだ根拠は欠けるのかなぁとはやはり思いますね。eslint-config-airbnbなどをはじめ、様々な選択肢があると思いますので、その中でなぜprettierを選択したのかな?と思った次第です。

ここはおっしゃるとおりです。確かに根拠に欠ける。実体験からの選択なので理解はちょっと甘いですが、一旦prettier使うとさせてください🙇