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

Add an option to clear the cache #755

Closed
regseb opened this issue Oct 2, 2022 · 4 comments · Fixed by #809
Closed

Add an option to clear the cache #755

regseb opened this issue Oct 2, 2022 · 4 comments · Fixed by #809
Labels

Comments

@regseb
Copy link
Contributor

regseb commented Oct 2, 2022

Feature request description

I want to use depcheck in a project. In the tests, I use mock-fs to mock files and test the integration of depcheck. But it's always the first values that are used by depcheck.

There should be an option to clear the cache.

Code snippets (if applicable)

package.json:

{
    "name": "testcase",
    "version": "1.0.0",
    "type": "module",
    "dependencies": {
        "depcheck": "1.4.3",
        "mock-fs": "5.2.0"
    }
}

index.js

import depcheck from "depcheck";
import mock from "mock-fs";

mock({
    "package.json": JSON.stringify({ dependencies: { foo: "1.0.0" } }),
});
console.log(await depcheck(process.cwd(), {}));
mock.restore();

mock({
    "package.json": JSON.stringify({ dependencies: { bar: "1.0.0" } }),
});
console.log(await depcheck(process.cwd(), {}));
mock.restore();

Any extra info

$ npm install

added 130 packages, and audited 131 packages in 46s

13 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
$ node index.js 
{
  dependencies: [ 'foo' ],
  devDependencies: [],
  missing: {},
  using: {},
  invalidFiles: {},
  invalidDirs: {}
}
{
  dependencies: [ 'foo' ],
  devDependencies: [],
  missing: {},
  using: {},
  invalidFiles: {},
  invalidDirs: {}
}
@regseb regseb added the feature label Oct 2, 2022
@SupervisionT
Copy link

I went through out the same issue and manged to clear the cache after extracting the dependencies
I'm not sure why the caching was used in the first place! but to solve this issue we have two options:
1- Always clear the cache after completing depcheck()
2- Add a new option of type boolean, clearCache, to depcheck. The option default value if not included is false.

If there is a good reason to keep the cache please let me know so I can go with option 2 otherwise I prefer to apply option 1

@znarf
Copy link
Collaborator

znarf commented Jun 8, 2023

What cache are you mentioning exactly? The promises stored in getContent perhaps? https://github.com/depcheck/depcheck/blob/main/src/utils/file.js

Sounds acceptable to flush it after completion.

It's an important piece of optimization, made depcheck multiple time faster on large codebases.

@regseb
Copy link
Contributor Author

regseb commented Jun 9, 2023

I used the word cache because I assume depcheck has a system for caching file contents, but I hadn't looked at the source code.

I would like to be able to call depcheck several times on the same file whose contents change between calls.


Here's a suggestion for emptying the contents of promises.

  • src/utils/file.js

    export function clearContent() {
      promises = {};
    }
  • src/constants.js

    export const defaultOptions = {
      // ...
      flush: false,
    };
  • src/index.js

    import { clearContent } from './utils/file';
    
    // ...
    
    export default function depcheck(rootDir, options, callback) {
      // ...
      const flush = getOption('flush');
      if (flush) {
          clearContent();
      }
      // ...
    }

@znarf
Copy link
Collaborator

znarf commented Jun 9, 2023

Looks good but as a matter of simplicity, I don't think we need the option and can simply clear it at the end.

The cache is not here for an hypothetical second run, it's here because we need to access the files several time in the same run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants