Skip to content

Commit

Permalink
caching is hard - make cacheKeys stable + add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@mikrostew
mikrostew committed Mar 16, 2018
1 parent 665201b commit d79d21d
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 34 deletions.
6 changes: 3 additions & 3 deletions lib/assets/AssetsCollection.js
View file
Expand Up @@ -57,9 +57,9 @@ AssetsCollection.prototype.asAssetImport = function (name) {
* @returns {String} the cache key
*/
AssetsCollection.prototype.cacheKey = function(name) {
return this.sources.reduce(function(cacheStr, source) {
return cacheStr + ":" + source.cacheKey(name);
}, "sources");
return this.sources.map(function(source) {
return source.cacheKey(name);
}).sort().join(":");
};

module.exports = AssetsCollection;
33 changes: 18 additions & 15 deletions lib/assets/AssetsSource.js
View file
Expand Up @@ -6,6 +6,7 @@ var path = require("path");
var merge = require("lodash.merge");
var URI = require("../util/URI");
var fileUtils = require("../util/files");
var stringify = require("json-stable-stringify");

/* class AssetsSource
*
Expand Down Expand Up @@ -82,31 +83,33 @@ AssetsSource.prototype.toString = function() {
return this.srcPath + "/" + this.pattern;
};

// don't include these globOpts in the cacheKey
function skipSomeKeys(key, value) {
// these are set to this.srcPath, which is already included in the cacheKey
if (key === "cwd" || key === "root") {
return undefined;
}
// these are added inside glob and always set to true, which happens after this string
// is created when glob.sync() is run in #getAssets()
// (see #setopts() in glob/common.js)
if (key === "nonegate" || key === "nocomment") {
return undefined;
}
return value;
}

/**
* Build a string suitable for caching an instance of this
* @returns {String} the cache key
*/
AssetsSource.prototype.cacheKey = function(namespace) {
function skipSomeKeys(key, value) {
// these are set to this.srcPath, which is already included in the cacheKey
if (key === "cwd" || key === "root") {
return undefined;
}
// these are added inside glob and always set to true, which happens after this string
// is created when glob.sync() is run in #getAssets()
// (see #setopts() in glob/common.js)
if (key === "nonegate" || key === "nocomment") {
return undefined;
}
return value;
}

return "[" +
"httpPrefix=" + (this.httpPrefix ? this.httpPrefix : "") +
";name=" + (this.name ? this.name : (namespace ? namespace : "")) +
";srcPath=" + this.srcPath +
";pattern=" + this.pattern +
";opts=" + JSON.stringify(this.globOpts, skipSomeKeys) +
// json-stable-stringify orders keys when stringifying (JSON.stringify does not)
";opts=" + stringify(this.globOpts, {replacer: skipSomeKeys}) +
"]";
};

Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Expand Up @@ -34,6 +34,7 @@
"ensure-symlink": "^1.0.0",
"fs-extra": "^0.30.0",
"glob": "^7.1.0",
"json-stable-stringify": "^1.0.1",
"lodash.includes": "^4.3.0",
"lodash.merge": "^4.6.0",
"node-sass": "^4.0.0 || ^3.10.1",
Expand Down
73 changes: 57 additions & 16 deletions test/test_assets.js
View file
Expand Up @@ -875,6 +875,26 @@ describe("assets", function () {
testutils.assertCompiles(eg, expected, done);
});

it("should keep track of module collections", function() {
var rootDir = testutils.fixtureDirectory("app_assets");

var egMod = new Eyeglass({
// file containing '@import "mod-one/assets"'
file: path.join(rootDir, "sass", "uses_mod_1.scss"),
eyeglass: {
root: rootDir,
installWithSymlinks: installWithSymlinks,
engines: {
sass: sass
}
}
});

assert.ok(egMod.assets.moduleCollections !== undefined);
assert.ok(Array.isArray(egMod.assets.moduleCollections));
assert.equal(egMod.assets.moduleCollections.length, 1);
});

describe("path separator normalization", function() {
var originalEnv = process.env.EYEGLASS_NORMALIZE_PATHS;
var merge = require("lodash.merge");
Expand Down Expand Up @@ -989,7 +1009,7 @@ describe("assets", function () {
var rootDir = testutils.fixtureDirectory("app_assets");
var rootDir2 = testutils.fixtureDirectory("app_assets_odd_names");

it("Assets.cacheKey includes httpPrefix", function(done) {
it("Assets.cacheKey includes httpPrefix", function() {
var source1 = new AssetsSource(rootDir, {
httpPrefix: "foo",
});
Expand All @@ -1001,10 +1021,9 @@ describe("assets", function () {
});
assert.equal(source1.cacheKey(), source2.cacheKey());
assert.notEqual(source1.cacheKey(), source3.cacheKey());
done();
});

it("Assets.cacheKey includes name", function(done) {
it("Assets.cacheKey includes name", function() {
var source1 = new AssetsSource(rootDir, {
name: "foo",
});
Expand All @@ -1016,27 +1035,24 @@ describe("assets", function () {
});
assert.equal(source1.cacheKey(), source2.cacheKey());
assert.notEqual(source1.cacheKey(), source3.cacheKey());
done();
});

it("Assets.cacheKey includes namespace", function(done) {
it("Assets.cacheKey includes namespace", function() {
var source1 = new AssetsSource(rootDir, {});
var source2 = new AssetsSource(rootDir, {});
assert.equal(source1.cacheKey("foo"), source2.cacheKey("foo"));
assert.notEqual(source1.cacheKey("foo"), source2.cacheKey("bar"));
done();
});

it("Assets.cacheKey includes srcPath", function(done) {
it("Assets.cacheKey includes srcPath", function() {
var source1 = new AssetsSource(rootDir, {});
var source2 = new AssetsSource(rootDir, {});
var source3 = new AssetsSource(rootDir2, {});
assert.equal(source1.cacheKey(), source2.cacheKey());
assert.notEqual(source1.cacheKey(), source3.cacheKey());
done();
});

it("Assets.cacheKey includes pattern", function(done) {
it("Assets.cacheKey includes pattern", function() {
var source1 = new AssetsSource(rootDir, {
pattern: "images/**/*",
});
Expand All @@ -1048,10 +1064,9 @@ describe("assets", function () {
});
assert.equal(source1.cacheKey(), source2.cacheKey());
assert.notEqual(source1.cacheKey(), source3.cacheKey());
done();
});

it("Assets.cacheKey includes globOpts", function(done) {
it("Assets.cacheKey includes globOpts", function() {
var source1 = new AssetsSource(rootDir, {});
var source2 = new AssetsSource(rootDir, {});
var source3 = new AssetsSource(rootDir, {
Expand All @@ -1061,10 +1076,9 @@ describe("assets", function () {
});
assert.equal(source1.cacheKey(), source2.cacheKey());
assert.notEqual(source1.cacheKey(), source3.cacheKey());
done();
});

it("Assets.cacheKey ignores globOpts that are overridden", function(done) {
it("Assets.cacheKey ignores globOpts that are overridden", function() {
var source1 = new AssetsSource(rootDir, {});
var source2 = new AssetsSource(rootDir, {
globOpts: {
Expand All @@ -1078,10 +1092,25 @@ describe("assets", function () {
});
assert.equal(source1.cacheKey(), source2.cacheKey());
assert.equal(source1.cacheKey(), source3.cacheKey());
done();
});

it("AssetsCollection.cacheKey includes collection sources", function(done) {
it("Assets.cacheKey globOpts order doesn't matter", function() {
var source1 = new AssetsSource(rootDir, {
globOpts: {
dot: true,
nonull: true
}
});
var source2 = new AssetsSource(rootDir, {
globOpts: {
nonull: true,
dot: true
}
});
assert.equal(source1.cacheKey(), source2.cacheKey());
});

it("AssetsCollection.cacheKey includes collection sources", function() {
var collection1 = new AssetsCollection();
var collection2 = new AssetsCollection();
var collection3 = new AssetsCollection();
Expand All @@ -1096,7 +1125,19 @@ describe("assets", function () {
assert.equal(collection1.cacheKey(), collection2.cacheKey());
assert.notEqual(collection1.cacheKey(), collection3.cacheKey());
assert.notEqual(collection1.cacheKey(), collection4.cacheKey());
done();
});

it("AssetsCollection.cacheKey source order doesn't matter", function() {
var collection1 = new AssetsCollection();
var collection2 = new AssetsCollection();

collection1.addSource(rootDir);
collection1.addSource(rootDir2);

collection2.addSource(rootDir2);
collection2.addSource(rootDir);

assert.equal(collection1.cacheKey(), collection2.cacheKey());
});

});
Expand Down

0 comments on commit d79d21d

Please sign in to comment.