Skip to content

Commit

Permalink
refactor(cache): simplify clean method (#358)
Browse files Browse the repository at this point in the history
* refactor(cache): simplify `clean` method

- `cache().clean()` is only called during instantiation in `options`, so we can instead just call it inside of the constructor
- there is no need to call `this.init()` in `clean` as it's only used in the constructor anyway, which already calls `this.init()`
  - and if `noCache` is `true`, `init` is basically a no-op too
    - so technically we don't need to call `init` _at all_ if `noCache`, but that's a larger refactor that I'm splitting into a separate commit/PR

- now that `clean` is just one conditional, we can invert it and return early instead
- it's also not necessary and less efficient to call `emptyDir` before `remove`; `remove` will unlink all the contents anyway
  - docs here: https://github.com/jprichardson/node-fs-extra/blob/0220eac966d7d6b9a595d69b1242ab8a397fba7f/docs/remove-sync.md
    - though this also just normal FS behavior
- IMO, it's also not necessary to check if it's a directory, we can `remove` it either way
  - and not necessary to log out if we _don't_ clean it
- then also just simplify the logic to use a `filter` instead of a nested `if`
  - which we already do in several places, so this follows existing code style

* undo dir and not clean log removal

- as requested in code review, will go with a safety-first operating assumption

- as such, with safety as modus operandus, make the logging more detailed in these scenarios, since they're not supposed to happen
  - and don't check code coverage on these as they're not supposed to happen in normal usage
  • Loading branch information
agilgur5 authored Jun 20, 2022
1 parent 9ea15ce commit fee9547
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 20 deletions.
3 changes: 0 additions & 3 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,6 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
noErrors = false;
}

if (pluginOptions.clean)
cache().clean();

return config;
},

Expand Down
43 changes: 26 additions & 17 deletions src/tscache.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as tsTypes from "typescript";
import { emptyDirSync, pathExistsSync, readdirSync, removeSync, statSync } from "fs-extra";
import * as fs from "fs-extra";
import * as _ from "lodash";
import { Graph, alg } from "graphlib";
import objHash from "object-hash";
Expand Down Expand Up @@ -127,6 +127,8 @@ export class TsCache
},
this.hashOptions,
)}`;
} else {
this.clean();
}

this.dependencyTree = new Graph({ directed: true });
Expand All @@ -146,26 +148,33 @@ export class TsCache
this.checkAmbientTypes();
}

public clean()
private clean()
{
if (pathExistsSync(this.cacheRoot))
if (!fs.pathExistsSync(this.cacheRoot))
return;

const entries = fs.readdirSync(this.cacheRoot);
entries.forEach((e) =>
{
const entries = readdirSync(this.cacheRoot);
entries.forEach((e) =>
const dir = `${this.cacheRoot}/${e}`;

/* istanbul ignore if -- this is a safety check, but shouldn't happen when using a dedicated cache dir */
if (!e.startsWith(this.cachePrefix))
{
const dir = `${this.cacheRoot}/${e}`;
if (e.startsWith(this.cachePrefix) && statSync(dir).isDirectory)
{
this.context.info(blue(`cleaning cache: ${dir}`));
emptyDirSync(`${dir}`);
removeSync(`${dir}`);
}
else
this.context.debug(`not cleaning ${dir}`);
});
}
this.context.debug(`skipping cleaning ${dir} as it does not have prefix ${this.cachePrefix}`);
return;
}

this.init();
/* istanbul ignore if -- this is a safety check, but should never happen in normal usage */
if (!fs.statSync(dir).isDirectory)
{
this.context.debug(`skipping cleaning ${dir} as it is not a directory`);
return;
}

this.context.info(blue(`cleaning cache: ${dir}`));
fs.removeSync(`${dir}`);
});
}

public setDependency(importee: string, importer: string): void
Expand Down

0 comments on commit fee9547

Please sign in to comment.