[Bug 38854] New: Prettier fails on ToolbarButton because of 'class' prop
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Bug ID: 38854 Summary: Prettier fails on ToolbarButton because of 'class' prop Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: ERM Assignee: jonathan.druart@gmail.com Reporter: jonathan.druart@gmail.com QA Contact: testopia@bugs.koha-community.org CC: jonathan.druart@gmail.com, jonathan.field@ptfs-europe.com, martin.renvoize@ptfs-europe.com, matt.blenkinsop@ptfs-europe.com, pedro.amorim@ptfs-europe.com Depends on: 34215 New version of prettier fails on ToolbarButton because of (I guess) what it thinks is a reserver keyword. [error] koha-tmpl/intranet-tmpl/prog/js/vue/components/ToolbarButton.vue: SyntaxError: Unexpected token, expected "{" (1:6) [error] > 1 | class [error] | ^ The error does not really make sense, the code is: class: { type: String, default: "btn btn-default", }, A solution could be to rename the property, but actually we never use another value than the default one. The easiest seems to simply remove this property. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34215 [Bug 34215] Vue Toolbar component should be more flexible -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 --- Comment #1 from Jonathan Druart <jonathan.druart@gmail.com> --- % prettier --version 3.4.2 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 --- Comment #2 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 176292 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176292&action=edit Bug 38854: Remove 'class' property from ToolbarButton New version of prettier fails on ToolbarButton because of (I guess) what it thinks is a reserver keyword. [error] koha-tmpl/intranet-tmpl/prog/js/vue/components/ToolbarButton.vue: SyntaxError: Unexpected token, expected "{" (1:6) [error] > 1 | class [error] | ^ The error does not really make sense, the code is: class: { type: String, default: "btn btn-default", }, A solution could be to rename the property, but actually we never use another value than the default one. The easiest seems to simply remove this property. % prettier --version 3.4.2 Test plan: Upgrade prettier (yarn add prettier), confirm you have a more recent version Try to tidy the file using `prettier koha-tmpl/intranet-tmpl/prog/js/vue/components/ToolbarButton.vue` Without this patch you get the "Unexpected token" error. With this patch apply you don't Also rebuild the dist files using `yarn js:build` and confirm that the buttons in the toolbar of the ERM and Preservation modules still appears and are styled like before. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |38664 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38664 [Bug 38664] Tidy the whole codebase -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Leo Stoyanov <leo.stoyanov@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |leo.stoyanov@bywatersolutio | |ns.com --- Comment #3 from Leo Stoyanov <leo.stoyanov@bywatersolutions.com> --- Hi Jonathan, After following the test plan, I wasn't able to recreate the bug. Specifically, even with the latest version of prettier (3.4.2) downloaded and even without the patch applied, there was no "SyntaxError: Unexpected token". Note that I am on a Windows 11 machine running WSL with an Ubuntu distribution. My development environment includes Docker Desktop for containerization, and the Koha testing environment is running inside WSL. Leo -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #4 from David Nind <david@davidnind.com> --- I attempted to test, but don't get the error before the patch. 1. I ran: yarn add prettier yarn add v1.22.22 [1/4] Resolving packages... [2/4] Fetching packages... [3/4] Linking dependencies... warning "@redocly/cli > redoc > @cfaester/enzyme-adapter-react-18@0.8.0" has unmet peer dependency "enzyme@^3.11.0". warning " > eslint-config-prettier@9.1.0" has unmet peer dependency "eslint@>=7.0.0". warning " > eslint-plugin-prettier@5.1.3" has unmet peer dependency "eslint@>=8.0.0". warning "swagger-cli > @apidevtools/swagger-cli > @apidevtools/swagger-parser@10.1.0" has unmet peer dependency "openapi-types@>=7". [4/4] Building fresh packages... success Saved lockfile. success Saved 1 new dependency. info Direct dependencies └─ prettier@3.4.2 info All dependencies └─ prettier@3.4.2 Done in 58.41s. 2. Prettier version after update: prettier --version 3.4.2 3. Output: prettier koha-tmpl/intranet-tmpl/prog/js/vue/components/ToolbarButton.vue <template> <router-link :to="to" :class="class" ><font-awesome-icon v-if="icon" :icon="icon" /> {{ title }}</router-link > </template> <script> export default { props: { to: { type: [String, Object], }, class: { type: String, default: "btn btn-default", }, icon: { type: String, required: false, }, title: { type: String, }, }, name: "Toolbar", } </script> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 --- Comment #5 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Same as the above - no errors. Are you using a prettier.config.js file in your branch with any additional config extending the defaults? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 --- Comment #6 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Or made any further changes in .prettierrc.js? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |RESOLVED Resolution|--- |INVALID --- Comment #7 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Matt Blenkinsop from comment #6)
Or made any further changes in .prettierrc.js?
Good catch! I indeed had an incorrectly-positioned config entry in .prettierrc.js (context is bug 38664). This is invalid, sorry for the noise! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 --- Comment #8 from Jonathan Druart <jonathan.druart@gmail.com> --- The patch can still be useful as it removes an unused property. What do you think Matt? Keeping for flexibility or removing to keep the code clean? It's also confusing to pass a value when it's actually the default value. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 --- Comment #9 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- I think we can remove it - if it needs adding back in at any point then we can make sure to use terminology that doesn't conflict with reserved keywords -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Prettier fails on |Unused 'class' prop in |ToolbarButton because of |ToolbarButton |'class' prop | Status|RESOLVED |REOPENED Resolution|INVALID |--- --- Comment #10 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Matt Blenkinsop from comment #9)
I think we can remove it - if it needs adding back in at any point then we can make sure to use terminology that doesn't conflict with reserved keywords
ok thanks for confirming. I have adjusted the bug title. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176292|0 |1 is obsolete| | --- Comment #11 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 176450 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176450&action=edit Bug 38854: Remove 'class' property from ToolbarButton We never use another value than the default one. We can simply remove this property. Test plan: Rebuild the dist files using `yarn js:build` and confirm that the buttons in the toolbar of the ERM and Preservation modules still appears and are styled like before. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 William Lavoie <william.lavoie@inLibro.com> 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=38854 William Lavoie <william.lavoie@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176450|0 |1 is obsolete| | --- Comment #12 from William Lavoie <william.lavoie@inLibro.com> --- Created attachment 176457 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176457&action=edit Bug 38854: Remove 'class' property from ToolbarButton We never use another value than the default one. We can simply remove this property. Test plan: Rebuild the dist files using `yarn js:build` and confirm that the buttons in the toolbar of the ERM and Preservation modules still appears and are styled like before. Signed-off-by: William Lavoie <william.lavoie@inLibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #13 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Looks like there are two errors: 1) AgreementsShow the class has been removed from an <a> tag that isn't part of the ToolbarButton component. The Delete button is styled incorrectly as a result 2) TrainsShow - the ToolbarButton component for editing trains has now got a '_class' property instead of having the 'class' removed (lines 93-101) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 --- Comment #14 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 176903 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176903&action=edit Bug 38854: Revert incorrect changes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 --- Comment #15 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Matt Blenkinsop from comment #13)
Looks like there are two errors: 1) AgreementsShow the class has been removed from an <a> tag that isn't part of the ToolbarButton component. The Delete button is styled incorrectly as a result
Oops, good catch!
2) TrainsShow - the ToolbarButton component for editing trains has now got a '_class' property instead of having the 'class' removed (lines 93-101)
Oops again, that was part of the first attempt. Thanks! I've added a follow-up patch to help the review, but please squash them when QAing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.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=38854 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176457|0 |1 is obsolete| | --- Comment #16 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Created attachment 176971 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176971&action=edit Bug 38854: Remove 'class' property from ToolbarButton We never use another value than the default one. We can simply remove this property. Test plan: Rebuild the dist files using `yarn js:build` and confirm that the buttons in the toolbar of the ERM and Preservation modules still appears and are styled like before. Signed-off-by: William Lavoie <william.lavoie@inLibro.com> Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 --- Comment #17 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Squashed and PQA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Patch doesn't apply --- Comment #18 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This no longer applies, can you please check? Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 38854: Revert incorrect changes error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementsShow.vue). error: could not build fake ancestor Patch failed at 0001 Bug 38854: Revert incorrect changes hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-38854-Revert-incorrect-changes-cicgpxvi.patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176903|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=38854 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Passed QA --- Comment #19 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Sorry, forgot to obsolete the squashed patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 --- Comment #20 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Try again, should be fine now -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |Main QA Contact|testopia@bugs.koha-communit |matt.blenkinsop@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 --- Comment #21 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- The dependecy is not even signed off yet :( -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 --- Comment #22 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Katrin Fischer from comment #21)
The dependecy is not even signed off yet :(
Obv wrong bug, please ignore. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |25.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 --- Comment #23 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 25.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=38854 --- Comment #24 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Part of the work done here, plus abstracting the component(s) to prevent things like these from being needed (change the same thing everywhere) has already been done in bug 37301 for many months. Any chance we can get some eyes on that bug? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|25.05.00 |25.05.00,24.11.04 released in| | Status|Pushed to main |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 --- Comment #25 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Nice work everyone! Pushed to 24.11.x for 24.11.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wainuiwitikapark@catalyst.n | |et.nz Status|Pushed to stable |Needs documenting --- Comment #26 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Depends on bug not in 22.11.x Not backporting to 22.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38854 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED --- Comment #27 from David Nind <david@davidnind.com> --- Bug fix, no changes to the manual required. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org