feat: relative cache support

[cs6cs6]

trafficstars

…velopers or into ci machines.

Summary

Related Issues

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: cs6cs6 (5f1c068facc3e36b9e66e23b7f9174677c3bf137, 159441603b08536ab9c217ac3fbc862f6e1e9cac, 4b483a91965e433f98b899c211f6374da6588ad7, bf15419edf12f3a712ecb2e510937ef14cc944e7, a09bbcf339c2e72fbf8bd3e740a7e6f63a3cb2ac, 0b7542c2a7eba7e8526f3429b39ec8d923ef0bfd, 0b5795c45dcfd89e8c872debb2e4eed986e665f0, 9ff6720c04ece10b4025d0dc7bc00b2302eade2d, c4597a71df74e87fcaafe8950519f3b39cb65a91, 88af1e7ec3686f4fa324f8c4860eeb7e18367d07, 1da9c6b14f85beb461c5885746d9470a48198d97)

[eslint-github-bot[bot]]

Hi @cs6cs6!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

• The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
• There should be a space following the initial tag and colon, for example 'feat: Message'.
• The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

[cs6cs6]

Hello. How do I draw attention to this PR? I believe I am only awaiting feedback.

[nzakas]

@cs6cs6 there were still some outstanding requests for you to consider here. Are you still working on this?

[nzakas]

@cs6cs6 just checking back to see if you intend on finishing this up?

[cs6cs6]

Hello, I apologize for the delay. I am working on this today.

[nzakas]

@bmish @mdjermanovic can you take another look at this?

[cs6cs6]

Hello. I am happy to answer further questions or clarify things. My team solved our critical build speed issue by removing a lot of our more costly linter rules, and running prettier outside of eslint instead of inside. But I would still like to be able to commit this change if you are interested. If you are not interested in this change, please let me know.

[nzakas]

Ping @bmish @mdjermanovic

[mdjermanovic]

I left a new comment on this conversation: https://github.com/eslint/rfcs/pull/114#discussion_r1372416892

[Rec0iL99]

Hey @jaredwray, could you please reformat your last https://github.com/eslint/rfcs/pull/114#issuecomment-1944423825.

[jaredwray]

Hey @jaredwray, could you please reformat your last #114 (comment).

Yep. Did above. Going to get tests around this over the next 72 hours and respond here on it.

[jaredwray]

@nzakas and @mdjermanovic let me know if you need any other changes or if you want file-entry-cache to work a different way with an example.

[nzakas]

@mdjermanovic I think you were the last one to look at file-entry-cache, do you recall if your concerns were addressed?

[jaredwray]

@mdjermanovic I think you were the last one to look at file-entry-cache, do you recall if your concerns were addressed?

@mdjermanovic just let me know anything you want changed and I can do it.

[mdjermanovic]

@jaredwray thanks for the support! I believe we'll need some changes or new features in file-entry-cache to support this use case. I'll take another look at the latest version of file-entry-cache and come up with an initial proposal in a day or two.

[mdjermanovic]

I see that the latest file-entry-cache v9 resolves relative paths passed to getFileDescriptor() as relative to process.cwd() and stores file keys as absolute paths in the cache file, as if the absolute paths were passed to getFileDescriptor().

What does everyone think about the following solution to support relative paths?

We're currently initializing cache this way:

this.fileEntryCache = fileEntryCache.create(
    cacheFileLocation,
    void 0,
    useChecksum
);

We could pass another argument, relative (boolean):

this.fileEntryCache = fileEntryCache.create(
    cacheFileLocation,
    void 0,
    useChecksum,
    relative
);

When relative is true, file-entry-cache operates in "relative" mode:

• File keys in the cache file are stored as relative paths (e.g., ./src/app.js or ../src/app.js), relative to the location of the cache file.
• getFileDescriptor() would still be getting absolute paths, but when searching for cache entries, it would convert them to relative paths.

[mdjermanovic]

Here a stackblitz demonstration of what we'd like to achieve:

https://stackblitz.com/edit/stackblitz-starters-axsbxs?file=package.json

[nzakas]

I see that the latest file-entry-cache v9 resolves relative paths passed to getFileDescriptor() as relative to process.cwd() and stores file keys as absolute paths in the cache file, as if the absolute paths were passed to getFileDescriptor().

I think hardcoding of process.cwd() is problematic as API users might set cwd to something else. Would it be possible to pass in the cwd?

[jaredwray]

I see that the latest file-entry-cache v9 resolves relative paths passed to getFileDescriptor() as relative to process.cwd() and stores file keys as absolute paths in the cache file, as if the absolute paths were passed to getFileDescriptor().

I think hardcoding of process.cwd() is problematic as API users might set cwd to something else. Would it be possible to pass in the cwd?

Yep, will get this added in this week!

[jaredwray]

Here a stackblitz demonstration of what we'd like to achieve:

https://stackblitz.com/edit/stackblitz-starters-axsbxs?file=package.json

awesome. Will work on this now and should have it released this week

[jaredwray]

Here a stackblitz demonstration of what we'd like to achieve: https://stackblitz.com/edit/stackblitz-starters-axsbxs?file=package.json

awesome. Will work on this now and should have it released this week

I did a pull request as I think we will want to just set the relative path at the constructor instead of passing in a boolean as we can check if it is an absolute path or not.

https://github.com/jaredwray/file-entry-cache/pull/52

Please let me know if this does the fix you want.

[mdjermanovic]

Here a stackblitz demonstration of what we'd like to achieve: https://stackblitz.com/edit/stackblitz-starters-axsbxs?file=package.json

awesome. Will work on this now and should have it released this week

I did a pull request as I think we will want to just set the relative path at the constructor instead of passing in a boolean as we can check if it is an absolute path or not.

jaredwray/file-entry-cache#52

Please let me know if this does the fix you want.

This doesn't seem to solve the problem, as the cache keys are still absolute paths. So when everything is moved to another location, the cache file becomes unusable because it contains entries for absolute paths that no longer exist, and the files that were cached now have different absolute paths.

[mdjermanovic]

@jaredwray just checking if you need more clarification on the use case we're trying to support.

[nzakas]

@jaredwray just checking in to see if @mdjermanovic's comments make sense?

[jaredwray]

@jaredwray just checking in to see if @mdjermanovic's comments make sense?

it does make sense and I am working on a new version that does not use absolute paths. Should be ready by the 19th.

[nzakas]

@jaredwray awesome. Thanks so much. We'll check back then.

[jaredwray]

@nzakas and @mdjermanovic - I released v9.1.0 of file-entry-cache it does what you are thinking but with one minor change. You need to specify the currentWorkingDir instead of just setting relativePath = true. The reason for this is that we are now using that path to handle the file keys correctly. You can see it working here with the example @mdjermanovic put together. https://stackblitz.com/edit/stackblitz-starters-npqlt7?file=package.json,test.js

const path = require('node:path');
const fileEntryCache = require('file-entry-cache');

const cacheFileLocation = path.resolve(__dirname, '.eslintcache');
const useChecksum = true;
const currentWorkingDirectory = __dirname;

const cache = fileEntryCache.create(
  cacheFileLocation,
  undefined,
  useChecksum,
  currentWorkingDirectory
);

Thanks for the patience on this and I believe this should un-block you with this where you want to rename the working folder and still use the cache.

[mdjermanovic]

@jaredwray thanks for the new versions!

Question: how to store file locations relative to the cache file?

I tried the following:

const fileEntryCache = require("file-entry-cache");
const path = require("node:path");

const cacheFileDir = path.resolve(__dirname, "cache");

const cacheFileLocation = path.resolve(cacheFileDir, ".eslintcache");
const useChecksum = true;
const currentWorkingDirectory = cacheFileDir;

const cache = fileEntryCache.create(
    cacheFileLocation,
    undefined,
    useChecksum,
    currentWorkingDirectory
);

const filePath = path.resolve(__dirname, "src", "my-file.js"); // exists
const fileDescriptor = cache.getFileDescriptor(filePath);
fileDescriptor.meta.results = { foo: "bar" };

cache.reconcile();

cache/.eslintcache cache file is successfully created, but it seems empty:

[{}]

[jaredwray]

@jaredwray thanks for the new versions!

Question: how to store file locations relative to the cache file?

I tried the following:

const fileEntryCache = require("file-entry-cache");
const path = require("node:path");

const cacheFileDir = path.resolve(__dirname, "cache");

const cacheFileLocation = path.resolve(cacheFileDir, ".eslintcache");
const useChecksum = true;
const currentWorkingDirectory = cacheFileDir;

const cache = fileEntryCache.create(
    cacheFileLocation,
    undefined,
    useChecksum,
    currentWorkingDirectory
);

const filePath = path.resolve(__dirname, "src", "my-file.js"); // exists
const fileDescriptor = cache.getFileDescriptor(filePath);
fileDescriptor.meta.results = { foo: "bar" };

cache.reconcile();

cache/.eslintcache cache file is successfully created, but it seems empty:

[{}]

Do you have a project that I can see with this? I believe the paths are causing issues on this and one thing to look at is that with using the currentWorkingDirectory you need to do relative path to the files. An example of this is the unit test here:

https://github.com/jaredwray/file-entry-cache/blob/0be8ea67cd55cafa277ac199a62c3f087396d517/test/relative.js#L71