[Bug 36400] New: Centralize {js, ts, vue} formatting config in .prettierrc.js
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Bug ID: 36400 Summary: Centralize {js,ts,vue} formatting config in .prettierrc.js Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: paul.derscheid@lmscloud.de QA Contact: testopia@bugs.koha-community.org 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. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 --- Comment #1 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Using the cli .editorconfig should be ignored by default, see https://github.com/prettier/prettier/issues/15401 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 --- Comment #2 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Created attachment 163695 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163695&action=edit 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_JavaSc...): [--trailing-comma es5, --arrow-parens avoid]. 4) Sign off if you're happy and comment or show what went wrong in a comment. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |blawlor@clamsnet.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 --- Comment #3 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 163710 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163710&action=edit 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_JavaSc...): [--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> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 --- Comment #4 from 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 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com --- Comment #5 from Jonathan Druart <jonathan.druart@gmail.com> --- Can you explain why we need eslint deps whereas we already have prettier? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |paul.derscheid@lmscloud.de |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 --- Comment #6 from Paul Derscheid <paul.derscheid@lmscloud.de> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163695|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163710|0 |1 is obsolete| | --- Comment #7 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 163799 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163799&action=edit 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_JavaSc...): [--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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Small patch |Trivial patch Status|Passed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 --- Comment #8 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Created attachment 163818 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163818&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 --- Comment #9 from Brendan Lawlor <blawlor@clamsnet.org> --- Is the test plan for the follow up the same as it was for the first patch? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 --- Comment #10 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Yes, it's exactly the same. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163818|0 |1 is obsolete| | --- Comment #11 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 163825 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163825&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 --- Comment #12 from 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! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163799|0 |1 is obsolete| | Attachment #163825|0 |1 is obsolete| | --- Comment #13 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 164248 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164248&action=edit 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_JavaSc...): [--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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 --- Comment #14 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 164249 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164249&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |jonathan.druart@gmail.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed, | |RM_priority --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- It looks like we are missing something here: t/Makefile.t not ok 1 - All directories should be mapped: .prettierrc.js -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Keywords|additional_work_needed, | |RM_priority | --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I have the test fix locally and will push shortly.. removing additional_work_needed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Needs documenting CC| |fridolin.somers@biblibre.co | |m --- Comment #18 from Fridolin Somers <fridolin.somers@biblibre.com> --- Not backported to 23.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36400 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED CC| |david@davidnind.com --- Comment #19 from David Nind <david@davidnind.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org