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
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.
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
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.
Created attachment 173070 [details] [review] Bug 38167: Remove remaining @vue/cli-service use
Created attachment 173071 [details] [review] Bug 38167: Make Prettier and ESLint happier for cypress/plugins/index.js
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>
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>
Created attachment 174431 [details] [review] Bug 38167: Remove remaining @vue/cli-service use Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
(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",
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.
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).
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.
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>
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>
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>
Created attachment 174858 [details] [review] Bug 38167: Remove remaining @vue/cli-service use Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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>
Rebased and split yarn.lock changes to their own commit.
(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)
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.
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.
How exactly does mockData.js need @vue/cli-service ? See bug 38503 comment 11
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>
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>
Created attachment 175429 [details] [review] Bug 38167: [DO NOT PUSH] yarn.lock changes Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 175430 [details] [review] Bug 38167: Remove remaining @vue/cli-service uses Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
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 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
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>
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>
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)
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>
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>
see comment on bug 38149.
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 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.
Pushed for 25.05! Well done everyone, thank you!
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