jaegertracing/jaeger-ui

Help with eslint upgrade

yurishkuro opened this issue · 19 comments

yurishkuro commented

#2271 is failing to bump eslint versions:

  1. We need to change our build to use Node 20 in .github/workflow/** and .nvmrc. It would be nice to enforce it via package.json.
  2. This error: ESLintIgnoreWarning: The ".eslintignore" file is no longer supported. Switch to using the "ignores" property in "eslint.config.js": https://eslint.org/docs/latest/use/configure/migration-guide#ignoring-files
  3. New error: [eslint ] Error: Could not find config file.
Baalekshan commented

Hi @yurishkuro , unit-tests workflow is failing in my commit, how can i resolve

yurishkuro commented

@Baalekshan the lint step is still failing after upgrade with

[eslint       ] Error: Could not find config file.
[eslint       ]     at locateConfigFileToUse (/home/runner/work/jaeger-ui/jaeger-ui/node_modules/eslint/lib/eslint/eslint.js:350:21)
[eslint       ]     at async calculateConfigArray (/home/runner/work/jaeger-ui/jaeger-ui/node_modules/eslint/lib/eslint/eslint.js:385:49)
[eslint       ]     at async ESLint.lintFiles (/home/runner/work/jaeger-ui/jaeger-ui/node_modules/eslint/lib/eslint/eslint.js:815:25)
[eslint       ]     at async Object.execute (/home/runner/work/jaeger-ui/jaeger-ui/node_modules/eslint/lib/cli.js:500:23)
[eslint       ]     at async main (/home/runner/work/jaeger-ui/jaeger-ui/node_modules/eslint/bin/eslint.js:153:22)
[eslint       ] error Command failed with exit code 2.
Baalekshan commented

@yurishkuro Is this a concern ? Since, it was safely merged without any errors in the workflow. I didn't find this error in the PR raised only faced this in my forked repo.

yurishkuro commented

@Baalekshan the change we merged was good, but it did not resolve this ticket, because the upgrade is still failing, just on a different error.

Baalekshan commented

Oh right, what could be done now. Should I try fixing these errors? If so what can I do.

yurishkuro commented

The upgrade is trying to bump eslint from 8.x to 9.x, there could be some breaking changes causing this issue.

Baalekshan commented

I will look into it

tico88612 commented

ESlint 9.x won't find .eslintrc.js in default.

There are two difference ways to resolve this.

  1. Env ESLINT_USE_FLAT_CONFIG set false
  2. .eslintrc.js migrate to eslint.config.js
  • 👍2
tico88612 commented

Unfortunately, there are still some packages that can't support flat config (eslint.config.js)

It may be necessary to re-filter the .eslintrc.js content.

Baalekshan commented

So do you mean we can't migrate completely but also use .eslint.config.js partially?

tico88612 commented

@Baalekshan
If you want to migrate settings, some of the packages will not work. (e.g. eslint-config-airbnb)

Set the environment variable ESLINT_USE_FLAT_CONFIG to false if you want to keep the full settings. (But I don't know about the possibility of .eslintrc.js being deprecated at some time.)

We can follow what @yurishkuro decides to do.

Baalekshan commented

@yurishkuro what should we do ?

yurishkuro commented

In the linked airbnb ticket there is some mentioning of compat workaround, maybe it could work. Otherwise we can wait till they support it. I don't see much point in using env var solution, if's probably temporary anyway.

  • 👍1
MiirzaBaig commented

Has anyone figured out the issue?

ayushrakesh commented

@yurishkuro above issue is solved or something is still wrong? Should I work on it.

Baalekshan commented

@MiirzaBaig @ayushrakesh There is a workaround but it's temporary. So, @yurishkuro suggested to wait until they support it.

yurishkuro commented

At this point I would go with a workaround, because airbnb ticket says it will take a while for them to fix due to dependencies.

ayushrakesh commented

ok 👍