Bug 38167 - ESLint: migrate config to flat format + cleanup some node dependencies
Summary: ESLint: migrate config to flat format + cleanup some node dependencies
Status: Pushed to main
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: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on: 37824 38149 38770
Blocks: 38524
  Show dependency treegraph
 
Reported: 2024-10-14 17:16 UTC by Victor Grousset/tuxayo
Modified: 2025-01-16 16:38 UTC (History)
4 users (show)

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


Attachments
Bug 38167: Migrate ESLint config to new flat format (2.77 KB, patch)
2024-10-14 18:21 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38167: Remove Babel which was only used by Webpack (175.14 KB, patch)
2024-10-14 18:21 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38167: Remove remaining @vue/cli-service use (780 bytes, patch)
2024-10-21 16:33 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38167: Make Prettier and ESLint happier for cypress/plugins/index.js (1.14 KB, patch)
2024-10-21 16:33 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38167: Migrate ESLint config to new flat format (2.83 KB, patch)
2024-11-12 17:20 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 38167: Remove Babel which was only used by Webpack (175.20 KB, patch)
2024-11-12 17:20 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 38167: Remove remaining @vue/cli-service use (839 bytes, patch)
2024-11-12 17:20 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 38167: Make Prettier and ESLint happier for cypress/plugins/index.js (1.20 KB, patch)
2024-11-12 17:20 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 38167: Migrate ESLint config to new flat format (2.83 KB, patch)
2024-11-20 18:59 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38167: Remove Babel which was only used by Webpack (1.62 KB, patch)
2024-11-20 18:59 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38167: [DO NOT PUSH] yarn.lock changes (175.17 KB, patch)
2024-11-20 18:59 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38167: Remove remaining @vue/cli-service use (839 bytes, patch)
2024-11-20 18:59 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38167: Make Prettier and ESLint happier for cypress/plugins/index.js (1.20 KB, patch)
2024-11-20 18:59 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38167: Migrate ESLint config to new flat format (2.83 KB, patch)
2024-12-13 03:49 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38167: Remove Babel which was only used by Webpack (1.62 KB, patch)
2024-12-13 03:49 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38167: [DO NOT PUSH] yarn.lock changes (174.44 KB, patch)
2024-12-13 03:50 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38167: Remove remaining @vue/cli-service uses (1.21 KB, patch)
2024-12-13 03:50 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38167: Make Prettier and ESLint happier for cypress/plugins/index.js (1.20 KB, patch)
2024-12-13 03:50 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38167: Migrate ESLint config to new flat format (3.29 KB, patch)
2024-12-22 10:37 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38167: Make Prettier and ESLint happier for cypress/plugins/index.js (1.20 KB, patch)
2024-12-22 10:37 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38167: Migrate ESLint config to new flat format (3.35 KB, patch)
2025-01-07 15:29 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38167: Make Prettier and ESLint happier for cypress/plugins/index.js (1.26 KB, patch)
2025-01-07 15:29 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38167: (QA follow-up) Remove old, add new eslint config to Makefile.PL (1.06 KB, patch)
2025-01-16 16:38 UTC, Paul Derscheid
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-10-14 17:16:55 UTC
ESLint9 switched to a new config file format[1]. Using the old config format looks like this:
ESLINT_USE_FLAT_CONFIG='false' eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js

So here we will be doing the migration like this:
npx @eslint/migrate-config .eslintrc.json
https://eslint.org/docs/latest/use/configure/migration-guide

[1] https://eslint.org/docs/latest/use/configure/configuration-files
    https://eslint.org/blog/2022/08/new-config-system-part-1/
    https://eslint.org/blog/2022/08/new-config-system-part-2/


-----


The goal of this is to make it easier to check guideline JS8

https://wiki.koha-community.org/wiki/Coding_Guidelines#JS8:_Follow_guidelines_set_by_ESLint

And be one step closer to able to check it on .vue and .ts files
Comment 1 Victor Grousset/tuxayo 2024-10-14 18:21:42 UTC
Created attachment 172754 [details] [review]
Bug 38167: Migrate ESLint config to new flat format

Test:
1. Do not apply
2. eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
3. It should fail
4. ESLINT_USE_FLAT_CONFIG='false' eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
5. It should work
6. Apply patches
7. Restart KTD to have a clean state of dependencies and check that
   provisionning still works.
8. ESLINT_USE_FLAT_CONFIG='false' eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
9. It should fail
10. eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
11. It should work
12 Double check rspack still works: rspack build
   Because we removed babel.
Comment 2 Victor Grousset/tuxayo 2024-10-14 18:21:45 UTC
Created attachment 172755 [details] [review]
Bug 38167: Remove Babel which was only used by Webpack

And our old versions was depending on a old buggy version of "globals"
that broke ESLint when using new config file format.
Removing Babel if it's not used anymore instead of updating it. (There
would be a way to test the updat works anyway.

@vue/cli-service was depending on babel core.
Removing it since it's deprecated (see replacements below) instead
having to keep a subset of Babel packages and updating them.
https://cli.vuejs.org/#getting-started
https://github.com/vuejs/vue-cli
Comment 3 Victor Grousset/tuxayo 2024-10-15 01:35:37 UTC
If this is merged as it is. A note will have to be left in bug 37824 because

> - we can always fallback onto webpack, it's not going anywhere.

To leave a trace of changes that relevant to an eventual fallback to webpack.
Comment 4 Victor Grousset/tuxayo 2024-10-21 16:33:14 UTC
Created attachment 173070 [details] [review]
Bug 38167: Remove remaining @vue/cli-service use
Comment 5 Victor Grousset/tuxayo 2024-10-21 16:33:16 UTC
Created attachment 173071 [details] [review]
Bug 38167: Make Prettier and ESLint happier for cypress/plugins/index.js
Comment 6 Paul Derscheid 2024-11-12 17:20:48 UTC
Created attachment 174429 [details] [review]
Bug 38167: Migrate ESLint config to new flat format

Test:
1. Do not apply
2. eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
3. It should fail
4. ESLINT_USE_FLAT_CONFIG='false' eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
5. It should work
6. Apply patches
7. Restart KTD to have a clean state of dependencies and check that
   provisionning still works.
8. ESLINT_USE_FLAT_CONFIG='false' eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
9. It should fail
10. eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
11. It should work
12 Double check rspack still works: rspack build
   Because we removed babel.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 7 Paul Derscheid 2024-11-12 17:20:51 UTC
Created attachment 174430 [details] [review]
Bug 38167: Remove Babel which was only used by Webpack

And our old versions was depending on a old buggy version of "globals"
that broke ESLint when using new config file format.
Removing Babel if it's not used anymore instead of updating it. (There
would be a way to test the updat works anyway.

@vue/cli-service was depending on babel core.
Removing it since it's deprecated (see replacements below) instead
having to keep a subset of Babel packages and updating them.
https://cli.vuejs.org/#getting-started
https://github.com/vuejs/vue-cli

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 8 Paul Derscheid 2024-11-12 17:20:53 UTC
Created attachment 174431 [details] [review]
Bug 38167: Remove remaining @vue/cli-service use

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 9 Paul Derscheid 2024-11-12 17:20:56 UTC
Created attachment 174432 [details] [review]
Bug 38167: Make Prettier and ESLint happier for cypress/plugins/index.js

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 10 Jonathan Druart 2024-11-13 09:38:09 UTC
(In reply to Paul Derscheid from comment #8)
> Created attachment 174431 [details] [review] [review]
> Bug 38167: Remove remaining @vue/cli-service use
> 
> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>

Isn't this occurrence of "vue-cli" related?

cypress.config.ts:            framework: "vue-cli",
Comment 11 Victor Grousset/tuxayo 2024-11-20 16:50:10 UTC
Good find, I guess it is.

Context:
```
    component: {
        devServer: {
            framework: "vue-cli",
            bundler: "webpack",
        },
```

What operation could use this config? Because just running the tests didn't use this and didn't fail. Whereas the stuff removed in the commit had to be removed to be able run the tests.


https://docs.cypress.io/app/component-testing/component-framework-configuration
> Custom Dev Server
> A custom function can be passed into the devServer option, which allows the use of build systems not provided by Cypress out of the box.

After this plus other search it's still blurry what is a dev server.
Comment 12 Paul Derscheid 2024-11-20 17:00:38 UTC
I think this is unused because we don't have component tests yet.

If I am not mistaken we just went e2e.

There's https://www.npmjs.com/package/cypress-rspack-dev-server (btw) but it doesn't support vue yet (just react).
Comment 13 Paul Derscheid 2024-11-20 17:02:32 UTC
But if we don't have any yet, I just want to casually advocate for adding component tests in the future.

Huge gains in actual test coverage for behaviour, comparatively little effort.
Comment 14 Victor Grousset/tuxayo 2024-11-20 18:59:36 UTC
Created attachment 174855 [details] [review]
Bug 38167: Migrate ESLint config to new flat format

Test:
1. Do not apply
2. eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
3. It should fail
4. ESLINT_USE_FLAT_CONFIG='false' eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
5. It should work
6. Apply patches
7. Restart KTD to have a clean state of dependencies and check that
   provisionning still works.
8. ESLINT_USE_FLAT_CONFIG='false' eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
9. It should fail
10. eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
11. It should work
12 Double check rspack still works: rspack build
   Because we removed babel.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 15 Victor Grousset/tuxayo 2024-11-20 18:59:38 UTC
Created attachment 174856 [details] [review]
Bug 38167: Remove Babel which was only used by Webpack

And our old versions was depending on a old buggy version of "globals"
that broke ESLint when using new config file format.
Removing Babel if it's not used anymore instead of updating it. (There
would be a way to test the updat works anyway.

@vue/cli-service was depending on babel core.
Removing it since it's deprecated (see replacements below) instead
having to keep a subset of Babel packages and updating them.
https://cli.vuejs.org/#getting-started
https://github.com/vuejs/vue-cli

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 16 Victor Grousset/tuxayo 2024-11-20 18:59:44 UTC
Created attachment 174857 [details] [review]
Bug 38167: [DO NOT PUSH] yarn.lock changes

And our old versions was depending on a old buggy version of "globals"
that broke ESLint when using new config file format.
Removing Babel if it's not used anymore instead of updating it. (There
would be a way to test the updat works anyway.

@vue/cli-service was depending on babel core.
Removing it since it's deprecated (see replacements below) instead
having to keep a subset of Babel packages and updating them.
https://cli.vuejs.org/#getting-started
https://github.com/vuejs/vue-cli

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 17 Victor Grousset/tuxayo 2024-11-20 18:59:46 UTC
Created attachment 174858 [details] [review]
Bug 38167: Remove remaining @vue/cli-service use

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 18 Victor Grousset/tuxayo 2024-11-20 18:59:49 UTC
Created attachment 174859 [details] [review]
Bug 38167: Make Prettier and ESLint happier for cypress/plugins/index.js

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 19 Victor Grousset/tuxayo 2024-11-20 19:03:58 UTC
Rebased and split yarn.lock changes to their own commit.
Comment 20 Victor Grousset/tuxayo 2024-11-21 18:57:50 UTC
(In reply to Paul Derscheid from comment #12)
> I think this is unused because we don't have component tests yet.
> 
> If I am not mistaken we just went e2e.
> 
> There's https://www.npmjs.com/package/cypress-rspack-dev-server (btw) but it
> doesn't support vue yet (just react).

Indeed: https://github.com/th3fallen/cypress-rspack-dev-server/issues/3


(In reply to Paul Derscheid from comment #13)
> But if we don't have any yet, I just want to casually advocate for adding
> component tests in the future.
> 
> Huge gains in actual test coverage for behaviour, comparatively little
> effort.

That would have been a reason to consider staying on webpack but as we see here, we could only know about cypress-rspack-dev-server existence and limitations after starting removing webpack dependencies. And thus making rolling back not a drop in replacement >_<


Should we just remove the `component` object from the config? Or leave it (as it's unused) so if someone wants to do component tests it's easier to adapt to cypress-rspack-dev-server? (whether it supports rspack or we have to come back to webpack)
Comment 21 Paul Derscheid 2024-11-21 20:16:27 UTC
I think we can remove it since it's unused and the default config, so we lose nothing (and it's in git anyway).

We could probably also use webpack for component tests (but rspack would probably be faster), since the bundling method is not thaaaaaaat relevant imo.

I also think it won't take forever for cypress to adopt the rspack dev server under their official namespace once support for vue, svelte and angular is added.
Comment 22 Paul Derscheid 2024-11-22 10:37:13 UTC
We can no longer remove it as we now need @vue/cli-service for mockData.js.

I could however resolve the issue by upgrading @vue/cli-service to its latests version (which depends on @achrinza/node-ipc@9.2.9 which is compatible with node v22.
Comment 23 Victor Grousset/tuxayo 2024-11-28 18:35:44 UTC
How exactly does mockData.js need @vue/cli-service ? See bug 38503 comment 11
Comment 24 Victor Grousset/tuxayo 2024-12-13 03:49:56 UTC
Created attachment 175427 [details] [review]
Bug 38167: Migrate ESLint config to new flat format

Test:
1. Do not apply
2. eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
3. It should fail
4. ESLINT_USE_FLAT_CONFIG='false' eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
5. It should work
6. Apply patches
7. Restart KTD to have a clean state of dependencies and check that
   provisionning still works.
8. ESLINT_USE_FLAT_CONFIG='false' eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
9. It should fail
10. eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
11. It should work
12 Double check rspack still works: rspack build
   Because we removed babel.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 25 Victor Grousset/tuxayo 2024-12-13 03:49:59 UTC
Created attachment 175428 [details] [review]
Bug 38167: Remove Babel which was only used by Webpack

And our old versions was depending on a old buggy version of "globals"
that broke ESLint when using new config file format.
Removing Babel if it's not used anymore instead of updating it. (There
would be a way to test the updat works anyway.

@vue/cli-service was depending on babel core.
Removing it since it's deprecated (see replacements below) instead
having to keep a subset of Babel packages and updating them.
https://cli.vuejs.org/#getting-started
https://github.com/vuejs/vue-cli

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 26 Victor Grousset/tuxayo 2024-12-13 03:50:07 UTC
Created attachment 175429 [details] [review]
Bug 38167: [DO NOT PUSH] yarn.lock changes

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 27 Victor Grousset/tuxayo 2024-12-13 03:50:10 UTC
Created attachment 175430 [details] [review]
Bug 38167: Remove remaining @vue/cli-service uses

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 28 Victor Grousset/tuxayo 2024-12-13 03:50:13 UTC
Created attachment 175431 [details] [review]
Bug 38167: Make Prettier and ESLint happier for cypress/plugins/index.js

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 29 Victor Grousset/tuxayo 2024-12-13 05:21:52 UTC
comment 10 addressed (another occurrence of "vue-cli" removed)

rebased on the updated bug 38149 patches that bumped prettier's version compared to when they were first submitted

eslint and rspack commands in test plan still work so doesn't look like that would invalidate the previous signoff.


--------------------------------


(In reply to Paul Derscheid from comment #22)
> We can no longer remove it as we now need @vue/cli-service for mockData.js.

Testing on bug 38503 and bug 38461 without @vue/cli-service worked so it seem we can keep the removal :D
Comment 30 Victor Grousset/tuxayo 2024-12-22 10:37:06 UTC
Created attachment 175897 [details] [review]
Bug 38167: Migrate ESLint config to new flat format

Test:
1. Do not apply
2. eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
3. It should fail
4. ESLINT_USE_FLAT_CONFIG='false' eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
5. It should work
6. Apply patches
     BZ38700 will conflict with BZ38149 on yarn.lock
     solve conflict with
     sudo yarn install --modules-folder /kohadevbox/node_modules
     git bz apply --continue
     git will only continue for 38700 and don't actually apply this (38167)
     So retry
     git bz apply 38167
     Don't apply the dependecies are they are already there.
     This time you're good :)
     (the conflict can't be durably solved because 38770 need to be
     applied independently or with BZ38149 depending on what other bug
     needs to be tested :/ )
7. Restart KTD to have a clean state of dependencies and check that
   provisionning still works.
8. ESLINT_USE_FLAT_CONFIG='false' eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
9. It should fail
10. eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
11. It should work

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 31 Victor Grousset/tuxayo 2024-12-22 10:37:09 UTC
Created attachment 175898 [details] [review]
Bug 38167: Make Prettier and ESLint happier for cypress/plugins/index.js

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 32 Victor Grousset/tuxayo 2024-12-22 10:40:52 UTC
Retested after the removal of @vue/cli-service and babel was moved to bug 38770.

Test plan got messier with the applying step but that doesn't seem to be avoidable with having node deps changes in bug 38770 that need to be applied here (so with bug 38149) or independently (because they are needed somewhere else without bug 38149)
Comment 33 Martin Renvoize (ashimema) 2025-01-07 15:29:04 UTC
Created attachment 176195 [details] [review]
Bug 38167: Migrate ESLint config to new flat format

Test:
1. Do not apply
2. eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
3. It should fail
4. ESLINT_USE_FLAT_CONFIG='false' eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
5. It should work
6. Apply patches
     BZ38700 will conflict with BZ38149 on yarn.lock
     solve conflict with
     sudo yarn install --modules-folder /kohadevbox/node_modules
     git bz apply --continue
     git will only continue for 38700 and don't actually apply this (38167)
     So retry
     git bz apply 38167
     Don't apply the dependecies are they are already there.
     This time you're good :)
     (the conflict can't be durably solved because 38770 need to be
     applied independently or with BZ38149 depending on what other bug
     needs to be tested :/ )
7. Restart KTD to have a clean state of dependencies and check that
   provisionning still works.
8. ESLINT_USE_FLAT_CONFIG='false' eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
9. It should fail
10. eslint koha-tmpl/intranet-tmpl/prog/js/ajax.js
11. It should work

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 34 Martin Renvoize (ashimema) 2025-01-07 15:29:07 UTC
Created attachment 176196 [details] [review]
Bug 38167: Make Prettier and ESLint happier for cypress/plugins/index.js

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 35 Katrin Fischer 2025-01-10 16:37:46 UTC
see comment on bug 38149.
Comment 36 Jonathan Druart 2025-01-16 14:31:19 UTC
Second patch is going to conflict with current main, do we really need it?

IMO it's more a job for the "tidy all" bug (it will be done automatically there).
Comment 37 Jonathan Druart 2025-01-16 14:37:34 UTC
Comment on attachment 176196 [details] [review]
Bug 38167: Make Prettier and ESLint happier for cypress/plugins/index.js

This is only tidying (I think!) and will be tidy later (bug 38664).

As it conflicts now we can just drop it.
Comment 38 Katrin Fischer 2025-01-16 14:57:02 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 39 Paul Derscheid 2025-01-16 16:38:24 UTC
Created attachment 176693 [details] [review]
Bug 38167: (QA follow-up) Remove old, add new eslint config to Makefile.PL

To test:
1) Run t/Makefile.t, observe failure
2) Apply patch
3) Run t/Makefile.t, observe success