lint has many error

[tolerance-go]

I'm confused, I pulled the code from the master branch, no changes, is it an environment problem? I'm not sure, I want to confirm your opinion

...
d 6   indent-legacy
  94:1   error  Expected indentation of 12 spaces but found 6  indent
  94:7   error  Expected indentation of 8 spaces but found 6   indent-legacy
  96:1   error  Expected indentation of 12 spaces but found 6  indent
  96:7   error  Expected indentation of 8 spaces but found 6   indent-legacy
  97:1   error  Expected indentation of 12 spaces but found 6  indent
  97:7   error  Expected indentation of 8 spaces but found 6   indent-legacy
  98:1   error  Expected indentation of 8 spaces but found 4   indent
  98:8   error  Missing semicolon                              semi
  99:1   error  Expected indentation of 4 spaces but found 2   indent

✖ 2591 problems (2591 errors, 0 warnings)
  2172 errors and 0 warnings potentially fixable with the `--fix` option.

env:

➜  impress git:(master) ✗ node -v
v18.12.0
➜  impress git:(master) ✗ npm -v
8.19.2
➜  impress git:(master) ✗ 

[fnogatz]

What command are you calling?

[tolerance-go]

What command are you calling?

npm run new-lint

---

If I run lint, I get the following feedback，Maybe I switch to some correct node version instead

➜ impress git:(master) ✗ npm run lint

impress.js@1.1.0 lint npm exec -- jshint src test/.js && npm exec -- jscs src test/.js

(node:93841) Warning: Accessing non-existent property 'padLevels' of module exports inside circular dependency (Use node --trace-warnings ... to show where the warning was created)

[fnogatz]

The code is currently formatted according to the (older) eslint configuration, so npm run lint should not yield any errors, even in the latest versions of node.js. It is called as well in npm run all.

The new-lint command appears to be a result of #722. Three years ago, @mohe2015 updated some of impress.js' dependencies. As part of this, they started to migrate over to jshint, but it was not yet taken into the code base. Quote:

Upgrading the linter causes lots of linting errors. I suggest you simply revert to the old linting for now, then send a separate PR that upgrades to eslint + makes all the fixes too.

Time to revive this idea? :)

[mohe2015]

jscs is replaced by eslint so I think that could definitely be removed. I don't know how useful jshint is.

In that PR I just updated the most important parts and definitely think somebody could bring this up to date again/further and make it pass the eslint checker. I'm not using impress.js currently so I don't intend to work on that.

You probably will need to decide on a code-style and then fix all the warnings and errors.

[fnogatz]

Thank you for the update and background information, @mohe2015! I'll wait for @henrikingo to review my other PRs first and will probably look into updating the dependencies as well as a second step.

[henrikingo]

If I remember correctly, eslint should supersede both jscs and jshint.

Apparently there isn't a ticket to switch to eslint, but as you can guess from the new-lint command, it was certainly the plan.

A general upgrade of dependencies is of course also overdue.

I would like to see both of those tasks worked on in their own issues. I will close this issue as it is technically not a bug, but did give rise to a good discussion,so thanks @tolerance-go for asking the question!