From 5f1c068facc3e36b9e66e23b7f9174677c3bf137 Mon Sep 17 00:00:00 2001 From: cs6cs6 Date: Thu, 5 Oct 2023 16:08:23 +0000 Subject: [PATCH 01/11] feat: This feature is to create an eslintcache that can be shared across developers or into ci machines. (#16493) --- designs/2023-relative-cache/README.md | 77 +++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 designs/2023-relative-cache/README.md diff --git a/designs/2023-relative-cache/README.md b/designs/2023-relative-cache/README.md new file mode 100644 index 00000000..85062d81 --- /dev/null +++ b/designs/2023-relative-cache/README.md @@ -0,0 +1,77 @@ + Repo: eslint/eslint +- Start Date: 2023-02-14 +- RFC PR: +- Authors: Christian Schulz (@cschulz)i and cs6cs6 + +# Relative cache location strategy + +## Summary + +To be able to share the created eslint cache between different systems f.e. local machine and CI, there is a need for a relative path stored in the cache. + +## Motivation + +The overall motivation for doing this is performance during the eslint process. + +Most people are running eslint on local developer machine and additional on the CI workers. +In high changing repositories there are several pull requests at the same time changing different files requiring linting all the time. +This is a time consuming process depending on the enabled rules like type checking rules. + +If the CI worker is able to cache its last run, it can just pick this cache up and use it. +Also developers can reuse this cache or commit it as part of a repository for the CI. + +## Detailed Design + +I have a branch with the following design already. It is not fully tested, so I'm not 100% sure this will be the final implementation but I think it is a good start. + +The LintResultCache takes a file path as parameter to find or store a file cache entry. + +One approach would be to truncate the given absolute path of the file by the path of the cache location. + +### Adding the command line parameter +- conf/default-cli-options.js: add the property 'shearableCache' with a default of false. It should be put in the section with the other cache variables, below cacheStrategy. +- docs/src/use/command-line-interface.md: Add an explanation of the 'shareable-cache' variable with the other cache variables: "By default, an eslintcache contains full file paths and thus cannot readily be shared across developers or ci machines. "False" is that default. "True" changes the internal storage to store a relative path to the process execution directory, thus making the eslintcache shareable across developers and ci machines. . If you change this setting, you must regenerate your entire eslint cache." +- eslint-helpers.js: add shareableCache variable (set to false) to the processOptions +- lib/options.js: Add an option 'shareable-cache' of type Boolean with a nice description for people to read: By default, an eslintcache contains full file paths and thus cannot readily be shared across developers or ci machines. "False" is that default. "True" changes the internal storage to store a relative path to the process execution directory, thus making the eslintcache shareable across developers and ci machines. . If you change this setting, you must regenerate your entire eslint cache. + + +### Changing cache file serialization +- lib/eslint/flat-eslint.js: Define the shearableCache property at the top, with the other properties, as a boolean +- lib/eslint/flat-eslint.js: Around line 873, where the flat-eslint.js file is saving the cache in the cachefile, if processOptions.shareableCache is set to true, pass a PARTIAL path to the lintResultCache.setCachedLintResults() function instead of the full path. + +## Documentation + +Add the new CLI flag to the CLI documentation. The new cli flag will be called shareable-cache. + +## Drawbacks + +Adding a new CLI flag adds some complexity for the end user. They will need to understand why the feature exists. + +Furthermore, defaulting to the existing behavior does help make upgrading less painful, but it is possible most people expect a shareable cache by default. This may cause confusion. + +## Backwards Compatibility Analysis + +The new flag, shareable-cache, passed in on the command line, will default to false. Unless people explicitly set it to true, then the old algorithms will +be used. They will not need to regenerate their cache unless they set this flag. + +## Alternatives + +A CLI tool which translates the existing cache to the current folder structure. + +## Open Questions + +No. + +## Help Needed + +I have a branch with code changes, but I am trying to figure out how to build an artifact to test with. Using the eslint.js artifact in the browser does not make sense +with a cache change, and I would like to test it in my project. How can I build a package that I can install with NPM into my project to test?? I am able to run the tests +on the eslintproject and they all pass. My fork is here: https://github.com/cs6cs6/eslint_patch/tree/fork/shareable-eslintcache + +## Frequently Asked Questions + +TBD + +## Related Discussions + +[Change Request: Eslintcache relative #16493](https://github.com/eslint/eslint/issues/16493) From 159441603b08536ab9c217ac3fbc862f6e1e9cac Mon Sep 17 00:00:00 2001 From: cs6cs6 Date: Wed, 11 Oct 2023 19:38:03 +0000 Subject: [PATCH 02/11] Improving introductory text for clarity based on feedback. --- designs/2023-relative-cache/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2023-relative-cache/README.md b/designs/2023-relative-cache/README.md index 85062d81..eb8df240 100644 --- a/designs/2023-relative-cache/README.md +++ b/designs/2023-relative-cache/README.md @@ -26,7 +26,7 @@ I have a branch with the following design already. It is not fully tested, so I' The LintResultCache takes a file path as parameter to find or store a file cache entry. -One approach would be to truncate the given absolute path of the file by the path of the cache location. +The suggested approach is to truncate the given absolute path of the file to a relative path in the cache. ### Adding the command line parameter - conf/default-cli-options.js: add the property 'shearableCache' with a default of false. It should be put in the section with the other cache variables, below cacheStrategy. From 4b483a91965e433f98b899c211f6374da6588ad7 Mon Sep 17 00:00:00 2001 From: cs6cs6 Date: Wed, 11 Oct 2023 19:46:56 +0000 Subject: [PATCH 03/11] Fixing typos --- designs/2023-relative-cache/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/designs/2023-relative-cache/README.md b/designs/2023-relative-cache/README.md index eb8df240..235dc04e 100644 --- a/designs/2023-relative-cache/README.md +++ b/designs/2023-relative-cache/README.md @@ -1,7 +1,7 @@ Repo: eslint/eslint - Start Date: 2023-02-14 - RFC PR: -- Authors: Christian Schulz (@cschulz)i and cs6cs6 +- Authors: Christian Schulz (@cschulz) and cs6cs6 # Relative cache location strategy @@ -29,7 +29,7 @@ The LintResultCache takes a file path as parameter to find or store a file cache The suggested approach is to truncate the given absolute path of the file to a relative path in the cache. ### Adding the command line parameter -- conf/default-cli-options.js: add the property 'shearableCache' with a default of false. It should be put in the section with the other cache variables, below cacheStrategy. +- conf/default-cli-options.js: add the property 'shareableCache' with a default of false. It should be put in the section with the other cache variables, below cacheStrategy. - docs/src/use/command-line-interface.md: Add an explanation of the 'shareable-cache' variable with the other cache variables: "By default, an eslintcache contains full file paths and thus cannot readily be shared across developers or ci machines. "False" is that default. "True" changes the internal storage to store a relative path to the process execution directory, thus making the eslintcache shareable across developers and ci machines. . If you change this setting, you must regenerate your entire eslint cache." - eslint-helpers.js: add shareableCache variable (set to false) to the processOptions - lib/options.js: Add an option 'shareable-cache' of type Boolean with a nice description for people to read: By default, an eslintcache contains full file paths and thus cannot readily be shared across developers or ci machines. "False" is that default. "True" changes the internal storage to store a relative path to the process execution directory, thus making the eslintcache shareable across developers and ci machines. . If you change this setting, you must regenerate your entire eslint cache. From bf15419edf12f3a712ecb2e510937ef14cc944e7 Mon Sep 17 00:00:00 2001 From: cs6cs6 Date: Wed, 11 Oct 2023 20:07:16 +0000 Subject: [PATCH 04/11] Adding sample cache information to the README to show what the old cache and the new cache would look like. --- designs/2023-relative-cache/README.md | 107 ++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/designs/2023-relative-cache/README.md b/designs/2023-relative-cache/README.md index 235dc04e..95e2c6d9 100644 --- a/designs/2023-relative-cache/README.md +++ b/designs/2023-relative-cache/README.md @@ -28,6 +28,113 @@ The LintResultCache takes a file path as parameter to find or store a file cache The suggested approach is to truncate the given absolute path of the file to a relative path in the cache. +### Example existing cache file + +Here is an example existing cache file for an extremely small project with two source files. You see that the full path of the file is included in the +filenames, '/home/USER/git/samplecode'. + +``` +[ + { + "/home/USER/git/samplecode/src/formatter.ts": "1", + "/home/USER/git/samplecode/src/vite-env.d.ts": "2" + }, + { + "size": 2584, + "mtime": 1695823503788, + "results": "3", + "hashOfConfig": "4" + }, + { + "size": 75, + "mtime": 1695814197580, + "results": "5", + "hashOfConfig": "4" + }, + { + "filePath": "6", + "messages": "7", + "suppressedMessages": "8", + "errorCount": 0, + "fatalErrorCount": 0, + "warningCount": 0, + "fixableErrorCount": 0, + "fixableWarningCount": 0 + }, + "1k0siqs", + { + "filePath": "9", + "messages": "10", + "suppressedMessages": "11", + "errorCount": 0, + "fatalErrorCount": 0, + "warningCount": 0, + "fixableErrorCount": 0, + "fixableWarningCount": 0 + }, + "/home/USER/git/samplecode/src/formatter.ts", + [], + [], + "/home/USER/git/samplecode/src/vite-env.d.ts", + [], + [] +] + +``` + +### Suggested updated cache file + +Here is a suggested updated cache file, with relative paths instead of full paths. The . is the directory from which +eslint is run. Typically, eslint is always run from the same place, so using a relative path should not cause a problem. + +``` +[ + { + "./src/formatter.ts": "1", + "./src/vite-env.d.ts": "2" + }, + { + "size": 2584, + "mtime": 1695823503788, + "results": "3", + "hashOfConfig": "4" + }, + { + "size": 75, + "mtime": 1695814197580, + "results": "5", + "hashOfConfig": "4" + }, + { + "filePath": "6", + "messages": "7", + "suppressedMessages": "8", + "errorCount": 0, + "fatalErrorCount": 0, + "warningCount": 0, + "fixableErrorCount": 0, + "fixableWarningCount": 0 + }, + "1k0siqs", + { + "filePath": "9", + "messages": "10", + "suppressedMessages": "11", + "errorCount": 0, + "fatalErrorCount": 0, + "warningCount": 0, + "fixableErrorCount": 0, + "fixableWarningCount": 0 + }, + "./src/formatter.ts", + [], + [], + "./src/vite-env.d.ts", + [], + [] +] +``` + ### Adding the command line parameter - conf/default-cli-options.js: add the property 'shareableCache' with a default of false. It should be put in the section with the other cache variables, below cacheStrategy. - docs/src/use/command-line-interface.md: Add an explanation of the 'shareable-cache' variable with the other cache variables: "By default, an eslintcache contains full file paths and thus cannot readily be shared across developers or ci machines. "False" is that default. "True" changes the internal storage to store a relative path to the process execution directory, thus making the eslintcache shareable across developers and ci machines. . If you change this setting, you must regenerate your entire eslint cache." From a09bbcf339c2e72fbf8bd3e740a7e6f63a3cb2ac Mon Sep 17 00:00:00 2001 From: cs6cs6 Date: Wed, 11 Oct 2023 20:18:19 +0000 Subject: [PATCH 05/11] Adding more information about a CLI Alternative to this code change, based on feedback. --- designs/2023-relative-cache/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2023-relative-cache/README.md b/designs/2023-relative-cache/README.md index 95e2c6d9..4c8c2d2d 100644 --- a/designs/2023-relative-cache/README.md +++ b/designs/2023-relative-cache/README.md @@ -163,7 +163,7 @@ be used. They will not need to regenerate their cache unless they set this flag. ## Alternatives -A CLI tool which translates the existing cache to the current folder structure. +A CLI tool which translates the existing cache to the current folder structure. This could be any sort of script that will basically do a replace-all on the paths in the .eslintcache file so that they are the new location rather than the old. A downside to this is that any tool based on string replacement would be fragile at best. ## Open Questions From 0b7542c2a7eba7e8526f3429b39ec8d923ef0bfd Mon Sep 17 00:00:00 2001 From: cs6cs6 Date: Tue, 17 Oct 2023 23:36:58 +0000 Subject: [PATCH 06/11] Updating RFC with improved design for shareableCache. --- designs/2023-relative-cache/README.md | 55 +++++++++++++++++++++------ 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/designs/2023-relative-cache/README.md b/designs/2023-relative-cache/README.md index 4c8c2d2d..1445614d 100644 --- a/designs/2023-relative-cache/README.md +++ b/designs/2023-relative-cache/README.md @@ -30,8 +30,11 @@ The suggested approach is to truncate the given absolute path of the file to a r ### Example existing cache file -Here is an example existing cache file for an extremely small project with two source files. You see that the full path of the file is included in the -filenames, '/home/USER/git/samplecode'. +Here is an example existing cache file for an extremely small project with two source files. It has been formatted for readability. + +You can see that the full path of the file is included in the filenames, '/home/USER/git/samplecode'. There is also an obscure field, the value +1k0siqs. That is the hash-value of a stringified ConfigArray object that contains all the configuration for the entire eslint run. It is there so +that any rule changes or configuration changes will cause a complete eslint recheck on all files. ``` [ @@ -87,6 +90,9 @@ filenames, '/home/USER/git/samplecode'. Here is a suggested updated cache file, with relative paths instead of full paths. The . is the directory from which eslint is run. Typically, eslint is always run from the same place, so using a relative path should not cause a problem. +Note that the obscure hash value is still there, and is now Ak0sovs. This hash value will need to be calculated using +relative paths only. + ``` [ { @@ -115,7 +121,7 @@ eslint is run. Typically, eslint is always run from the same place, so using a r "fixableErrorCount": 0, "fixableWarningCount": 0 }, - "1k0siqs", + "Ak0sovs", { "filePath": "9", "messages": "10", @@ -138,13 +144,24 @@ eslint is run. Typically, eslint is always run from the same place, so using a r ### Adding the command line parameter - conf/default-cli-options.js: add the property 'shareableCache' with a default of false. It should be put in the section with the other cache variables, below cacheStrategy. - docs/src/use/command-line-interface.md: Add an explanation of the 'shareable-cache' variable with the other cache variables: "By default, an eslintcache contains full file paths and thus cannot readily be shared across developers or ci machines. "False" is that default. "True" changes the internal storage to store a relative path to the process execution directory, thus making the eslintcache shareable across developers and ci machines. . If you change this setting, you must regenerate your entire eslint cache." -- eslint-helpers.js: add shareableCache variable (set to false) to the processOptions +- eslint-helpers.js: add shareableCache variable (set to false) to the processOptions, and make sure it must be a boolean. - lib/options.js: Add an option 'shareable-cache' of type Boolean with a nice description for people to read: By default, an eslintcache contains full file paths and thus cannot readily be shared across developers or ci machines. "False" is that default. "True" changes the internal storage to store a relative path to the process execution directory, thus making the eslintcache shareable across developers and ci machines. . If you change this setting, you must regenerate your entire eslint cache. - +- lib/cli.js: Add the shareableCache option, defaulted to false, in the translateOptions function. ### Changing cache file serialization -- lib/eslint/flat-eslint.js: Define the shearableCache property at the top, with the other properties, as a boolean -- lib/eslint/flat-eslint.js: Around line 873, where the flat-eslint.js file is saving the cache in the cachefile, if processOptions.shareableCache is set to true, pass a PARTIAL path to the lintResultCache.setCachedLintResults() function instead of the full path. +- lib/cli-engine/lint-result-cache.js: Add the properties 'cwd' and 'shareableCache' to the LintResultCache class. 'cwd' is a string, the current working directory. shareableCache is a boolean, the result of the user's passed in command line parameter. Use these values in two main places: + 1. Whenever we store or retrieve file results in the cache, use the results of the new class function, getFileKey(filePath). If shareableCache is false, it uses a full file path as the key. If shareableCache is true, uses a relative file path. + 2. Update the function hashOfConfigFor, so that when the configuration object is hashed into a hash key and put into the eslintcache, if shareableCache is set to true, the file paths hashed are all relative paths. Implementation detail: +``` +function hashOfConfigFor(config, cwd) { + if (!configHashCache.has(config)) { + // replace the full directory path in the config string to make the hash location-neutral + const stringConfig = stringify(config).replaceAll(cwd, '.'); + configHashCache.set(config, hash(`${pkg.version}_${nodeVersion}_${stringConfig}`)); + } +``` +- lib/cli-engine/cli-engine.js: Update the constructor call to LintResultCache to add the new parameters cwd and shareableCache. +- lib/eslint/flat-eslint.js: Update the constructor call to LintResultCache to add the new parameters cwd and shareableCache. ## Documentation @@ -159,11 +176,14 @@ Furthermore, defaulting to the existing behavior does help make upgrading less p ## Backwards Compatibility Analysis The new flag, shareable-cache, passed in on the command line, will default to false. Unless people explicitly set it to true, then the old algorithms will -be used. They will not need to regenerate their cache unless they set this flag. +be used. Users of the eslint cache will still need to regenerate their cache no matter how they set this flag, because the current implementation forces a cache rebuild if there are +any configuration changes. Adding a new property counts as a change. ## Alternatives -A CLI tool which translates the existing cache to the current folder structure. This could be any sort of script that will basically do a replace-all on the paths in the .eslintcache file so that they are the new location rather than the old. A downside to this is that any tool based on string replacement would be fragile at best. +A CLI tool which translates the existing cache to the current folder structure. This could be any sort of script that will basically do a replace-all on the paths in +the .eslintcache file and also recalculates the hash so that they are the new location rather than the old. A downside to this is that any tool based on string +replacement would be fragile at best. ## Open Questions @@ -171,9 +191,20 @@ No. ## Help Needed -I have a branch with code changes, but I am trying to figure out how to build an artifact to test with. Using the eslint.js artifact in the browser does not make sense -with a cache change, and I would like to test it in my project. How can I build a package that I can install with NPM into my project to test?? I am able to run the tests -on the eslintproject and they all pass. My fork is here: https://github.com/cs6cs6/eslint_patch/tree/fork/shareable-eslintcache +Decision needed: Make every cache shareable, or keep the shreable-cache flag default to false? + +I originally planned the shareable-cache flag so that users of eslint could not be surprised by changed cache behavior, and would also not have to regenerate their cache +for a patch version change. But I discovered by testing that there is no way to NOT force people to regenerate their eslint cache with this change. That's because by adding +the 'shareable-cache' flag, it adds a property to the ConfigArray object. Even if it's set to false (the old behavior), the object's structure changes with a new property +field. This in turn changes the ConfigArray object's stringify results, which are fed into a hash function to create a hash key. (See lint-result-cache.js function hashOfConfigFor.) +In turn, that becomes a part of the eslintcache file. I think it's so that everything will be re-scanned with configuration changes. + +For that reason, should we simply have a shareable cache full of relative paths be the only behavior? That means no command line flag, and strictly internal changes that +would be invisible to the end user. On the other hand, if this change DOES have undesirable side effects because people are using eslint in unexpected ways, having the flag +default to the old behavior would save them from surprises on upgrade. Judging by the number of .gitignore files with .eslintcache in them, people are used to this +cache not being shareable. + +What would the eslint team prefer? No flag, or keep the shareable-cache flag and default to false? ## Frequently Asked Questions From 0b5795c45dcfd89e8c872debb2e4eed986e665f0 Mon Sep 17 00:00:00 2001 From: cs6cs6 Date: Wed, 25 Oct 2023 12:28:34 +0000 Subject: [PATCH 07/11] Accepting feedback comments. --- designs/2023-relative-cache/README.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/designs/2023-relative-cache/README.md b/designs/2023-relative-cache/README.md index 1445614d..dc4640bc 100644 --- a/designs/2023-relative-cache/README.md +++ b/designs/2023-relative-cache/README.md @@ -87,8 +87,8 @@ that any rule changes or configuration changes will cause a complete eslint rech ### Suggested updated cache file -Here is a suggested updated cache file, with relative paths instead of full paths. The . is the directory from which -eslint is run. Typically, eslint is always run from the same place, so using a relative path should not cause a problem. +Here is a suggested updated cache file, with relative paths instead of full paths. The `.` is the current working +directory. Typically, eslint is always run from the same place, so using a relative path should not cause a problem. Note that the obscure hash value is still there, and is now Ak0sovs. This hash value will need to be calculated using relative paths only. @@ -152,12 +152,11 @@ relative paths only. - lib/cli-engine/lint-result-cache.js: Add the properties 'cwd' and 'shareableCache' to the LintResultCache class. 'cwd' is a string, the current working directory. shareableCache is a boolean, the result of the user's passed in command line parameter. Use these values in two main places: 1. Whenever we store or retrieve file results in the cache, use the results of the new class function, getFileKey(filePath). If shareableCache is false, it uses a full file path as the key. If shareableCache is true, uses a relative file path. 2. Update the function hashOfConfigFor, so that when the configuration object is hashed into a hash key and put into the eslintcache, if shareableCache is set to true, the file paths hashed are all relative paths. Implementation detail: -``` +```js function hashOfConfigFor(config, cwd) { if (!configHashCache.has(config)) { // replace the full directory path in the config string to make the hash location-neutral - const stringConfig = stringify(config).replaceAll(cwd, '.'); - configHashCache.set(config, hash(`${pkg.version}_${nodeVersion}_${stringConfig}`)); + // implementation TBD; please note configHashCache is a complex object with over 100 fields. } ``` - lib/cli-engine/cli-engine.js: Update the constructor call to LintResultCache to add the new parameters cwd and shareableCache. From 9ff6720c04ece10b4025d0dc7bc00b2302eade2d Mon Sep 17 00:00:00 2001 From: cs6cs6 <130847767+cs6cs6@users.noreply.github.com> Date: Wed, 25 Oct 2023 08:30:12 -0400 Subject: [PATCH 08/11] Update designs/2023-relative-cache/README.md Accepting code review suggestion where backticks are added for readability. Co-authored-by: Nicholas C. Zakas --- designs/2023-relative-cache/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/designs/2023-relative-cache/README.md b/designs/2023-relative-cache/README.md index dc4640bc..b0cea024 100644 --- a/designs/2023-relative-cache/README.md +++ b/designs/2023-relative-cache/README.md @@ -149,9 +149,9 @@ relative paths only. - lib/cli.js: Add the shareableCache option, defaulted to false, in the translateOptions function. ### Changing cache file serialization -- lib/cli-engine/lint-result-cache.js: Add the properties 'cwd' and 'shareableCache' to the LintResultCache class. 'cwd' is a string, the current working directory. shareableCache is a boolean, the result of the user's passed in command line parameter. Use these values in two main places: - 1. Whenever we store or retrieve file results in the cache, use the results of the new class function, getFileKey(filePath). If shareableCache is false, it uses a full file path as the key. If shareableCache is true, uses a relative file path. - 2. Update the function hashOfConfigFor, so that when the configuration object is hashed into a hash key and put into the eslintcache, if shareableCache is set to true, the file paths hashed are all relative paths. Implementation detail: +- `lib/cli-engine/lint-result-cache.js`: Add the properties `cwd` and `shareableCache` to the `LintResultCache` class. `cwd` is a string, the current working directory. `shareableCache` is a boolean, the result of the user's passed in command line parameter. Use these values in two main places: + 1. Whenever we store or retrieve file results in the cache, use the results of the new class function, `getFileKey(filePath)`. If `shareableCache` is false, it uses a full file path as the key. If `shareableCache` is true, uses a relative file path. + 2. Update the function `hashOfConfigFor`, so that when the configuration object is hashed into a hash key and put into the eslintcache, if `shareableCache` is set to true, the file paths hashed are all relative paths. Implementation detail: ```js function hashOfConfigFor(config, cwd) { if (!configHashCache.has(config)) { From c4597a71df74e87fcaafe8950519f3b39cb65a91 Mon Sep 17 00:00:00 2001 From: cs6cs6 <130847767+cs6cs6@users.noreply.github.com> Date: Wed, 25 Oct 2023 08:31:17 -0400 Subject: [PATCH 09/11] Update designs/2023-relative-cache/README.md Per code review, adding backticks for readability. Co-authored-by: Nicholas C. Zakas --- designs/2023-relative-cache/README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/designs/2023-relative-cache/README.md b/designs/2023-relative-cache/README.md index b0cea024..7ed4bf18 100644 --- a/designs/2023-relative-cache/README.md +++ b/designs/2023-relative-cache/README.md @@ -142,11 +142,11 @@ relative paths only. ``` ### Adding the command line parameter -- conf/default-cli-options.js: add the property 'shareableCache' with a default of false. It should be put in the section with the other cache variables, below cacheStrategy. -- docs/src/use/command-line-interface.md: Add an explanation of the 'shareable-cache' variable with the other cache variables: "By default, an eslintcache contains full file paths and thus cannot readily be shared across developers or ci machines. "False" is that default. "True" changes the internal storage to store a relative path to the process execution directory, thus making the eslintcache shareable across developers and ci machines. . If you change this setting, you must regenerate your entire eslint cache." -- eslint-helpers.js: add shareableCache variable (set to false) to the processOptions, and make sure it must be a boolean. -- lib/options.js: Add an option 'shareable-cache' of type Boolean with a nice description for people to read: By default, an eslintcache contains full file paths and thus cannot readily be shared across developers or ci machines. "False" is that default. "True" changes the internal storage to store a relative path to the process execution directory, thus making the eslintcache shareable across developers and ci machines. . If you change this setting, you must regenerate your entire eslint cache. -- lib/cli.js: Add the shareableCache option, defaulted to false, in the translateOptions function. +- `conf/default-cli-options.js`: add the property `shareableCache` with a default of false. It should be put in the section with the other cache variables, below `cacheStrategy`. +- `docs/src/use/command-line-interface.md`: Add an explanation of the `shareable-cache` variable with the other cache variables: "By default, an eslintcache contains full file paths and thus cannot readily be shared across developers or ci machines. "False" is that default. "True" changes the internal storage to store a relative path to the process execution directory, thus making the eslintcache shareable across developers and ci machines. . If you change this setting, you must regenerate your entire eslint cache." +- `eslint-helpers.js`: add `shareableCache` variable (set to false) to the `processOptions`, and make sure it must be a boolean. +- `lib/options.js`: Add an option `shareable-cache` of type Boolean with a nice description for people to read: By default, an eslintcache contains full file paths and thus cannot readily be shared across developers or ci machines. "False" is that default. "True" changes the internal storage to store a relative path to the process execution directory, thus making the eslintcache shareable across developers and ci machines. . If you change this setting, you must regenerate your entire eslint cache. +- `lib/cli.js`: Add the `shareableCache` option, defaulted to false, in the `translateOptions `function. ### Changing cache file serialization - `lib/cli-engine/lint-result-cache.js`: Add the properties `cwd` and `shareableCache` to the `LintResultCache` class. `cwd` is a string, the current working directory. `shareableCache` is a boolean, the result of the user's passed in command line parameter. Use these values in two main places: From 88af1e7ec3686f4fa324f8c4860eeb7e18367d07 Mon Sep 17 00:00:00 2001 From: cs6cs6 Date: Wed, 25 Oct 2023 12:42:50 +0000 Subject: [PATCH 10/11] Per code review comment, answering Help Needed item and moving it to Open Questions. --- designs/2023-relative-cache/README.md | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/designs/2023-relative-cache/README.md b/designs/2023-relative-cache/README.md index 7ed4bf18..85dfb6f5 100644 --- a/designs/2023-relative-cache/README.md +++ b/designs/2023-relative-cache/README.md @@ -186,24 +186,13 @@ replacement would be fragile at best. ## Open Questions -No. +Question: Should we make every cache shareable, or keep the shreable-cache flag default to false? -## Help Needed - -Decision needed: Make every cache shareable, or keep the shreable-cache flag default to false? +Answer: We don't know how people are currently using the cache file, so we can't make assumptions about what is and isn't safe to change. Even if we made a shareable cache the default (which would be a breaking change), we'd still need a way to enable the previous behavior in case there is a use case we aren't aware of.. -I originally planned the shareable-cache flag so that users of eslint could not be surprised by changed cache behavior, and would also not have to regenerate their cache -for a patch version change. But I discovered by testing that there is no way to NOT force people to regenerate their eslint cache with this change. That's because by adding -the 'shareable-cache' flag, it adds a property to the ConfigArray object. Even if it's set to false (the old behavior), the object's structure changes with a new property -field. This in turn changes the ConfigArray object's stringify results, which are fed into a hash function to create a hash key. (See lint-result-cache.js function hashOfConfigFor.) -In turn, that becomes a part of the eslintcache file. I think it's so that everything will be re-scanned with configuration changes. - -For that reason, should we simply have a shareable cache full of relative paths be the only behavior? That means no command line flag, and strictly internal changes that -would be invisible to the end user. On the other hand, if this change DOES have undesirable side effects because people are using eslint in unexpected ways, having the flag -default to the old behavior would save them from surprises on upgrade. Judging by the number of .gitignore files with .eslintcache in them, people are used to this -cache not being shareable. +## Help Needed -What would the eslint team prefer? No flag, or keep the shareable-cache flag and default to false? +None at this time. ## Frequently Asked Questions From 1da9c6b14f85beb461c5885746d9470a48198d97 Mon Sep 17 00:00:00 2001 From: cs6cs6 Date: Fri, 12 Jan 2024 15:22:58 +0000 Subject: [PATCH 11/11] Removing references to now defunct flat-eslint.js --- designs/2023-relative-cache/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/designs/2023-relative-cache/README.md b/designs/2023-relative-cache/README.md index 85dfb6f5..8f71aa75 100644 --- a/designs/2023-relative-cache/README.md +++ b/designs/2023-relative-cache/README.md @@ -160,7 +160,6 @@ function hashOfConfigFor(config, cwd) { } ``` - lib/cli-engine/cli-engine.js: Update the constructor call to LintResultCache to add the new parameters cwd and shareableCache. -- lib/eslint/flat-eslint.js: Update the constructor call to LintResultCache to add the new parameters cwd and shareableCache. ## Documentation