Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESLint ignores its own cache #13517

Closed
kirill-konshin opened this issue Jul 21, 2020 · 7 comments
Closed

ESLint ignores its own cache #13517

kirill-konshin opened this issue Jul 21, 2020 · 7 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion auto closed The bot closed this issue works as intended The behavior described in this issue is working correctly

Comments

@kirill-konshin
Copy link

kirill-konshin commented Jul 21, 2020

Tell us about your environment

We have a Gitlab configured this way:

cache:
  key: $CI_COMMIT_REF_NAME
  paths:
    - .eslint

lint:
  stage: test
  script: DEBUG=eslint:cli-engine yarn lint:all

And package.json:

{
  "scripts": {
    "lint": "eslint --cache --cache-location .eslint/cache --fix --ignore-path .eslintignore",
    "lint:all": "yarn lint \"src/**/*.{js,jsx,ts,tsx}\"",
  }
}
  • ESLint Version: 7.2.0

What did you expect to happen?

I can see that the ESLint cache is being successfully created on previous job launches, e.g. a file .eslint/cache is present in Gitlab Cache archive. And I expect to see that cache is used:

  eslint:cli-engine Skipping file since it hasn't changed: /xxx/src/app/core/data/stores/utils.js +0ms

What actually happened? Please include the actual, raw output from ESLint.

Looks like the cache is not used because I see output like this (for ALL the files in the repo):

  eslint:cli-engine Lint /xxx/src/app/core/data/stores/utils.js +132ms
                    ^^^^
                    this means cache was not used

But like I said, I do see the ESLint cache file being extracted from the Gitlab's cache.

Main question to ESLint is what can cause the cache file to be ignored?

Are you willing to submit a pull request to fix this bug?

Yes, if someone points where to look.

@kirill-konshin kirill-konshin added bug ESLint is working incorrectly triage An ESLint team member will look at this issue soon labels Jul 21, 2020
@mdjermanovic
Copy link
Member

Hi @kirill-konshin, thanks for the issue!

Main question to ESLint is what can cause the cache file to be ignored?

If you are using the same cache file in different clones of the same repository, it's more likely that the cache file itself is used but all the files have different mtimes compared to the run where the cache file was created.

ESLint cache currently uses mtime and size of the file to verify if the file was changed. There is eslint/rfcs#63 proposal to optionally use checksum instead of mtime.

@kirill-konshin
Copy link
Author

kirill-konshin commented Jul 21, 2020

We are using $CI_COMMIT_REF_NAME which is a branch, cache file is cached per-branch. The problem is that Git does not adjust mtimes https://stackoverflow.com/a/1964508, you’re right.

Which in turn means that cache is useless. In our case it takes 15min to lint w/o a cache... I’ve managed to speed it up to 4min by using https://github.com/jest-community/jest-runner-eslint to run ESLint in parallel. Fun part is that it has its own cache issue on top of ESLint’s one jest-community/jest-runner-eslint#82.

So basically at this moment I don’t see how to utilize cache in the CI env like Gitlab where workspace can be cloned over and over. Unlike some other CI systems, where workspace is created per-branch and runner/executor is sticky. In Gitlab any job can be picked up by any runner.

P.S. What a coincidence, the proposal you were referring to was created 2 days ago )

@mdjermanovic
Copy link
Member

Unfortunately, at the moment it does seem useless in this scenario. I hope we'll be able to add better support for the use of eslint cache on CIs in some of the future versions.

@mdjermanovic
Copy link
Member

Marking as "works as intended" since it isn't a bug in the actual implementation.

I think we should improve this, probably by adding an option (e.g., as proposed in eslint/rfcs#63).

As for possible workarounds, it might be useful to track jaredwray/file-entry-cache#14 (looks like it might be merged soon). It will allow for setting an env variable FILE_ENTRY_CACHE_USE_CHECKSUM which would be used directly from the file-entry-cache dependency.

@mdjermanovic mdjermanovic added works as intended The behavior described in this issue is working correctly and removed bug ESLint is working incorrectly triage An ESLint team member will look at this issue soon labels Jul 22, 2020
@eslint-deprecated eslint-deprecated bot added the auto closed The bot closed this issue label Aug 23, 2020
@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

@mockdeep
Copy link
Contributor

We're running into this same issue. We restore the cache on CircleCI, but it gets ignored. Would love an option to use the file hash for caching instead of mtime.

@mdjermanovic
Copy link
Member

We're running into this same issue. We restore the cache on CircleCI, but it gets ignored. Would love an option to use the file hash for caching instead of mtime.

We are working on that now (--cache-strategy content option per eslint/rfcs#63).

PR to follow: #13713

@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Mar 18, 2021
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Mar 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion auto closed The bot closed this issue works as intended The behavior described in this issue is working correctly
Projects
None yet
Development

No branches or pull requests

3 participants