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
Created attachment 176292 [details] [review] 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.
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
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>
Same as the above - no errors. Are you using a prettier.config.js file in your branch with any additional config extending the defaults?
Or made any further changes in .prettierrc.js?
(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!
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.
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
(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.
Created attachment 176450 [details] [review] 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.
Created attachment 176457 [details] [review] 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>
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)