Skip to content

Commit

Permalink
[Bugfix] Ensure stability of filename cache-keys
Browse files Browse the repository at this point in the history
`JSON.stringify(structure)` isn’t inherently stable as it relies on
various internal details of how `structure` was created.

As written, if a given babel configuration is create in an dynamic
manner, it is possible for babel-loader to have spurious cache misses.

To address this, we can use one of the many stable stringify
alternatives.

For this PR I have selected
[fast-stable-stringify](https://www.npmjs.com/package/fast-stable-stringify)
for that task, as it appears both popular and it’s benchmarks look
  promising.

This PR does not explicitly include tests, as testing this is both
tricky to test in this context, and the important tests are contained
within fast-stable-stringify itself.
  • Loading branch information
stefanpenner committed Jun 11, 2021
1 parent 7fdf6f4 commit fde5dfd
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 5 deletions.
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -10,6 +10,7 @@
"node": ">= 8.9"
},
"dependencies": {
"fast-stable-stringify": "^1.0.0",
"find-cache-dir": "^3.3.1",
"loader-utils": "^1.4.0",
"make-dir": "^3.1.0",
Expand Down
5 changes: 2 additions & 3 deletions src/cache.js
Expand Up @@ -24,6 +24,7 @@ const writeFile = promisify(fs.writeFile);
const gunzip = promisify(zlib.gunzip);
const gzip = promisify(zlib.gzip);
const makeDir = require("make-dir");
const stringify = require("fast-stable-stringify");

/**
* Read the contents from the compressed file.
Expand Down Expand Up @@ -65,9 +66,7 @@ const write = async function (filename, compress, result) {
const filename = function (source, identifier, options) {
const hash = crypto.createHash("md4");

const contents = JSON.stringify({ source, options, identifier });

hash.update(contents);
hash.update(stringify({ source, options, identifier }));

return hash.digest("hex") + ".json";
};
Expand Down
3 changes: 2 additions & 1 deletion src/index.js
Expand Up @@ -28,6 +28,7 @@ const schema = require("./schema");
const { isAbsolute } = require("path");
const loaderUtils = require("loader-utils");
const validateOptions = require("schema-utils");
const stringify = require("fast-stable-stringify");

function subscribe(subscriber, metadata, context) {
if (context[subscriber]) {
Expand Down Expand Up @@ -186,7 +187,7 @@ async function loader(source, inputSourceMap, overrides) {

const {
cacheDirectory = null,
cacheIdentifier = JSON.stringify({
cacheIdentifier = stringify({
options,
"@babel/core": transform.version,
"@babel/loader": version,
Expand Down
2 changes: 1 addition & 1 deletion test/cache.test.js
Expand Up @@ -376,7 +376,7 @@ test.cb("should allow to specify the .babelrc file", t => {

fs.readdir(t.context.cacheDirectory, (err, files) => {
t.is(err, null);
t.true(files.length === 2);
t.true(files.length === 1);
t.end();
});
});
Expand Down
8 changes: 8 additions & 0 deletions yarn.lock
Expand Up @@ -2140,6 +2140,7 @@ __metadata:
eslint-config-prettier: ^6.3.0
eslint-plugin-flowtype: ^5.2.0
eslint-plugin-prettier: ^3.0.0
fast-stable-stringify: ^1.0.0
find-cache-dir: ^3.3.1
husky: ^4.3.0
lint-staged: ^10.5.1
Expand Down Expand Up @@ -3732,6 +3733,13 @@ __metadata:
languageName: node
linkType: hard

"fast-stable-stringify@npm:^1.0.0":
version: 1.0.0
resolution: "fast-stable-stringify@npm:1.0.0"
checksum: 972291ed8b8a1a1e13130c91062852dc9142dceed14dc1655c1d0aebc07adcf8a60678252dd1410ec0cceddd3842c87cfea2a937ee3043dfaa6068c3f578f515
languageName: node
linkType: hard

"fastq@npm:^1.6.0":
version: 1.9.0
resolution: "fastq@npm:1.9.0"
Expand Down

0 comments on commit fde5dfd

Please sign in to comment.