I think a prettier config file is the way to go here. We need to ignore all formatting rules of eslint or we might get performance issues. Please correct component if wrong.
Using the cli .editorconfig should be ignored by default, see https://github.com/prettier/prettier/issues/15401
Created attachment 163695 [details] [review] Attachment to Bug 36400 - Centralize {js,ts,vue} formatting config in .prettierrc.js Bug 36400: Centralize {js,ts,vue} formatting config in .prettierrc.js Let's centralize all formatting for js, ts and vue files in a .prettierrc.js. The goal here is to preserve eslint's linting but ignore all of its formatting options. Seems to work for vue files in vscode, vscodium and neovim w/ vscode-eslint-language-server. To test: 1) Pick any vue file for example. 2) Run format via your editor and either npx prettier -c .prettierrc.js <PATH_TO_VUE_FILE> [you can also use the -w flag to directly write]. 3) Look for unexpected changes: vue files should be already formatted w/ these inline flags (see: https://wiki.koha-community.org/wiki/Coding_Guidelines#JS14:_Prettier_JavaScript): [--trailing-comma es5, --arrow-parens avoid]. 4) Sign off if you're happy and comment or show what went wrong in a comment.
Created attachment 163710 [details] [review] Bug 36400: Centralize {js,ts,vue} formatting config in .prettierrc.js Let's centralize all formatting for js, ts and vue files in a .prettierrc.js. The goal here is to preserve eslint's linting but ignore all of its formatting options. Seems to work for vue files in vscode, vscodium and neovim w/ vscode-eslint-language-server. To test: 1) Pick any vue file for example. 2) Run format via your editor and either npx prettier -c .prettierrc.js <PATH_TO_VUE_FILE> [you can also use the -w flag to directly write]. 3) Look for unexpected changes: vue files should be already formatted w/ these inline flags (see: https://wiki.koha-community.org/wiki/Coding_Guidelines#JS14:_Prettier_JavaScript): [--trailing-comma es5, --arrow-parens avoid]. 4) Sign off if you're happy and comment or show what went wrong in a comment. Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Tested using vscode: Before the patch: npx prettier ./koha-tmpl/intranet-tmpl/prog/js/vue/components/KohaTable.vue git diff --compact-summary ./koha-tmpl/ intranet-tmpl/prog/js/vue/components/KohaTable.vue .../prog/js/vue/components/KohaTable.vue | 110 +++++++++---------- 1 file changed, 55 insertions(+), 55 deletions(-) After the patch: npx prettier -c .prettierrc.js -w ./koha-tmpl/intranet-tmpl/prog/js/vue/components/KohaTable.vue Checking formatting... [warn] Ignored unknown option { editorconfig: "false" }. [warn] Ignored unknown option { editorconfig: "false" }. All matched files use Prettier code style! No changes made to the vue file
Can you explain why we need eslint deps whereas we already have prettier?
Yes, using eslint and prettier together can cause performance problems because if not mitigated they both apply their formatting rules which can cause very sluggish formatting and even timeouts.
Created attachment 163799 [details] [review] Bug 36400: Centralize {js,ts,vue} formatting config in .prettierrc.js Let's centralize all formatting for js, ts and vue files in a .prettierrc.js. The goal here is to preserve eslint's linting but ignore all of its formatting options. Seems to work for vue files in vscode, vscodium and neovim w/ vscode-eslint-language-server. To test: 1) Pick any vue file for example. 2) Run format via your editor and either npx prettier -c .prettierrc.js <PATH_TO_VUE_FILE> [you can also use the -w flag to directly write]. 3) Look for unexpected changes: vue files should be already formatted w/ these inline flags (see: https://wiki.koha-community.org/wiki/Coding_Guidelines#JS14:_Prettier_JavaScript): [--trailing-comma es5, --arrow-parens avoid]. 4) Sign off if you're happy and comment or show what went wrong in a comment. Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 163818 [details] [review] Bug 36400 (follow-up): remove option editorconfig from .prettierrc.js - The option doesn't seem do what it's intended for: - The debug logs still show options from the .editorconfig (which is actually what we want in this case). - The option causes unwanted warnings when using the cli. - In summary: we have to keep in mind that the rules are now hierarchical: 1. .editorconfig 2. .prettierrc.js
Is the test plan for the follow up the same as it was for the first patch?
Yes, it's exactly the same.
Created attachment 163825 [details] [review] Bug 36400 (follow-up): remove option editorconfig from .prettierrc.js - The option doesn't seem do what it's intended for: - The debug logs still show options from the .editorconfig (which is actually what we want in this case). - The option causes unwanted warnings when using the cli. - In summary: we have to keep in mind that the rules are now hierarchical: 1. .editorconfig 2. .prettierrc.js Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Before patch: npx prettier -w ./koha-tmpl/intranet-tmpl/prog/js/vue/components/KohaTable.vue git diff --compact-summary ./koha-tmpl/ intranet-tmpl/prog/js/vue/components/KohaTable.vue .../prog/js/vue/components/KohaTable.vue | 110 +++++++++---------- 1 file changed, 55 insertions(+), 55 deletions(-) After patch: npx prettier -c .prettierrc.js -w ./koha-tmpl/intranet-tmpl/prog/js/vue/components/KohaTable.vue Checking formatting... All matched files use Prettier code style!
Created attachment 164248 [details] [review] Bug 36400: Centralize {js,ts,vue} formatting config in .prettierrc.js Let's centralize all formatting for js, ts and vue files in a .prettierrc.js. The goal here is to preserve eslint's linting but ignore all of its formatting options. Seems to work for vue files in vscode, vscodium and neovim w/ vscode-eslint-language-server. To test: 1) Pick any vue file for example. 2) Run format via your editor and either npx prettier -c .prettierrc.js <PATH_TO_VUE_FILE> [you can also use the -w flag to directly write]. 3) Look for unexpected changes: vue files should be already formatted w/ these inline flags (see: https://wiki.koha-community.org/wiki/Coding_Guidelines#JS14:_Prettier_JavaScript): [--trailing-comma es5, --arrow-parens avoid]. 4) Sign off if you're happy and comment or show what went wrong in a comment. Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 164249 [details] [review] Bug 36400 (follow-up): remove option editorconfig from .prettierrc.js - The option doesn't seem do what it's intended for: - The debug logs still show options from the .editorconfig (which is actually what we want in this case). - The option causes unwanted warnings when using the cli. - In summary: we have to keep in mind that the rules are now hierarchical: 1. .editorconfig 2. .prettierrc.js Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed for 24.05! Well done everyone, thank you!
It looks like we are missing something here: t/Makefile.t not ok 1 - All directories should be mapped: .prettierrc.js
I have the test fix locally and will push shortly.. removing additional_work_needed.
Not backported to 23.11.x
No changes required to the manual. If changes are required to the coding guidelines (https://wiki.koha-community.org/wiki/Coding_Guidelines), please update them.