Bug 38524 - Add support for Vue.js and TypeScript to ESLint config to be able to actually enforce coding guideline JS8
Summary: Add support for Vue.js and TypeScript to ESLint config to be able to actually...
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Victor Grousset/tuxayo
QA Contact: Testopia
URL:
Keywords:
Depends on: 38167
Blocks:
  Show dependency treegraph
 
Reported: 2024-11-24 18:52 UTC by Victor Grousset/tuxayo
Modified: 2024-11-25 04:43 UTC (History)
0 users

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 38524: Enable ESLint for .ts and .vue files (4.93 KB, patch)
2024-11-25 00:11 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38524: [DO NOT PUSH] yarn.lock changes (28.47 KB, patch)
2024-11-25 00:11 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Victor Grousset/tuxayo 2024-11-24 18:52:01 UTC
Currently ESLint doesn't try to check .vue and .ts files.
Comment 1 Victor Grousset/tuxayo 2024-11-25 00:11:07 UTC
Created attachment 174963 [details] [review]
Bug 38524: Enable ESLint for .ts and .vue files

Test plan:
1. Apply only bug 38167 and it's dependency (assuming they are not in main)
2. Restart KTD or run
     sudo yarn install --modules-folder /kohadevbox/node_modules
3. Run the following and see that it doesn't lint
     eslint t/cypress/integration/AdditionalFields_spec.ts
     eslint koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/UsageStatisticsReportBuilder.vue
4. Apply the patches of this ticket
5. restart KTD
   (it tests that the provisionning works well to get all
   the JS libs)
6. Run the following and see that it lints
     eslint t/cypress/integration/AdditionalFields_spec.ts
     eslint ./t/cypress/integration/ERM/DataProviders_spec.ts
     eslint ./koha-tmpl/intranet-tmpl/prog/js/vue/modules/preservation.ts
     eslint koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/UsageStatisticsReportBuilder.vue
     eslint ./koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementsFormAdd.vue

Various files tested here give an idea of the output, feel free to try others.
And report stuff that should obviously be silenced due to making too much
noise and having low value or being contrary to our current practices.
Non obvious stuff to fix might be put in a follow-up ticket.

---------

This was removed from config:
indent: ["error", 4],

Because this check was finding it redundant:
npx eslint-config-prettier koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/Main.vue
check which is documented here:
https://github.com/prettier/eslint-config-prettier?tab=readme-ov-file#cli-helper-tool

To doubled check, I sabotaged some indentation in a .js/.ts section on these files
koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/Main.vue
koha-tmpl/intranet-tmpl/prog/js/ajax.js
And without `indent: ["error", 4]` the prettier rule was still reporting the issue.
And with `indent: ["error", 4]` the issue was reported twice.
This looks enough to confirm it's redundant to keep it.

----

For the same reason as above, the following was removed:
semi: ["error", "always"],
tested by removing a semicolon in
koha-tmpl/intranet-tmpl/prog/js/ajax.js
t/cypress/integration/Auth/csrf.ts

----

This was removed:
"linebreak-style": ["error", "unix"],

Because it's the default of prettier
https://prettier.io/docs/en/options.html#end-of-line
Comment 2 Victor Grousset/tuxayo 2024-11-25 00:11:11 UTC
Created attachment 174964 [details] [review]
Bug 38524: [DO NOT PUSH] yarn.lock changes
Comment 3 Victor Grousset/tuxayo 2024-11-25 02:42:16 UTC
Things that look interesting by I'm not sure if lint rules should be changed or not:
eslint koha-tmpl/intranet-tmpl/prog/js/vue/components/AdditionalFieldsDisplay.vue
  33:9   warning  Prop "resource_type" is not in camelCase                             vue/prop-name-casing
  34:9   warning  Prop "additional_field_values" is not in camelCase                   vue/prop-name-casing
Comment 4 Victor Grousset/tuxayo 2024-11-25 04:43:45 UTC
Bonus of this: includes also prettier (guideline JS14)

As for "to be able to actually enforce coding guideline JS8".
There is a fair amount of tuning and deliberation before being to be able to use eslint for .vue and .ts like we are supposed to do for .js .

So even with these patches we might want to amend JS8 to clarify that while Typescript and Vue files are covered by it, there are too much checks that are not relevant for us or pending work on clarifying our guidelines. So we can't by default ask everything to be followed.