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 module discovery caching per #187 #189

Merged
merged 4 commits into from Oct 5, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/assets/Assets.js
Expand Up @@ -124,7 +124,8 @@ Assets.prototype.resolveAsset = function($assetsMap, $uri, cb) {
cb(error);
} else {
if (file) {
debug.assets(
/* istanbul ignore next - don't test debug */
debug.assets && debug.assets(
"%s resolved to %s with URI %s",
originalUri,
path.relative(options.root, file),
Expand Down
1 change: 1 addition & 0 deletions lib/importers/FSImporter.js
Expand Up @@ -20,6 +20,7 @@ function FSImporter(eyeglass, sass, options, fallbackImporter) {
} else {
absolutePath = path.resolve(path.dirname(prev));
}
/* istanbul ignore else - TODO: revisit this */
if (absolutePath) {
var sassContents = '@import "eyeglass/fs"; @include fs-register-path('
+ identifier + ', "' + absolutePath + '");';
Expand Down
3 changes: 2 additions & 1 deletion lib/importers/ImportUtilities.js
Expand Up @@ -28,7 +28,8 @@ ImportUtilities.createImporter = function(importer) {
ImportUtilities.prototype.importOnce = function(data, done) {
if (this.options.eyeglass.enableImportOnce && this.context.eyeglass.imported[data.file]) {
// log that we've already imported this file
debug.import("%s was already imported", data.file);
/* istanbul ignore next - don't test debug */
debug.import && debug.import("%s was already imported", data.file);
done({contents: "", file: "already-imported:" + data.file});
} else {
this.context.eyeglass.imported[data.file] = true;
Expand Down
154 changes: 79 additions & 75 deletions lib/modules/EyeglassModules.js
Expand Up @@ -3,6 +3,7 @@
var resolve = require("../util/resolve");
var packageUtils = require("../util/package");
var EyeglassModule = require("./EyeglassModule");
var SimpleCache = require("../util/SimpleCache");
var debug = require("../util/debug");
var path = require("path");
var merge = require("lodash.merge");
Expand Down Expand Up @@ -31,7 +32,8 @@ function EyeglassModules(dir, modules) {
};

this.cache = {
access: {}
access: new SimpleCache(),
modules: new SimpleCache()
};

// find the nearest package.json for the given directory
Expand Down Expand Up @@ -70,7 +72,8 @@ function EyeglassModules(dir, modules) {
// check for any issues we may have encountered
checkForIssues.call(this);

debug.modules("discovered modules\n\t" + this.getGraph().replace(/\n/g, "\n\t"));
/* istanbul ignore next - don't test debug */
debug.modules && debug.modules("discovered modules\n\t" + this.getGraph().replace(/\n/g, "\n\t"));
}

/**
Expand Down Expand Up @@ -257,39 +260,34 @@ function canAccessModule(name, origin) {
var canAccessFrom = function canAccessFrom(origin) {
// find the nearest package for the origin
var pkg = packageUtils.findNearestPackage(origin);

var cache = this.cache.access;
cache[name] = cache[name] || {};

// check for cached result
if (cache[name][pkg] !== undefined) {
return cache[name][pkg];
}

// find all the branches that match the origin
var branches = findBranchesByPath({
dependencies: this.tree
}, pkg);

var canAccess = branches.some(function(branch) {
// if the reference is to itself (branch.name)
// OR it's an immediate dependency (branch.dependencies[name])
if (branch.name === name || branch.dependencies && branch.dependencies[name]) {
return true;
}
var cacheKey = JSON.stringify({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stringify isn't stable by default, the cache key may not result in collisions due to this.

would ${origin}|${pkg.name}|${pkg.version} be sufficient, or better yet realpath of that location?

If not, maybe the following lib may be good.

pkg: pkg,
origin: origin
});

debug.import(
"%s can%s be imported from %s",
name,
(canAccess ? "" : "not"),
origin
);

// cache it for future lookups
cache[name][pkg] = canAccess;

return canAccess;
return this.cache.access.getOrElse(cacheKey, function() {
// find all the branches that match the origin
var branches = findBranchesByPath({
dependencies: this.tree
}, pkg);

var canAccess = branches.some(function(branch) {
// if the reference is to itself (branch.name)
// OR it's an immediate dependency (branch.dependencies[name])
if (branch.name === name || branch.dependencies && branch.dependencies[name]) {
return true;
}
});

/* istanbul ignore next - don't test debug */
debug.import && debug.import(
"%s can%s be imported from %s",
name,
(canAccess ? "" : "not"),
origin
);
return canAccess;
}.bind(this));
}.bind(this);

// check if we can access from the origin...
Expand Down Expand Up @@ -346,7 +344,8 @@ function getDependencyVersionIssues(modules, finalModule) {
return modules.map(function(mod) {
// if the versions are not identical, log it
if (mod.version !== finalModule.version) {
debug.modules(
/* istanbul ignore next - don't test debug */
debug.modules && debug.modules(
"asked for %s@%s but using %s",
mod.name,
mod.version,
Expand Down Expand Up @@ -454,53 +453,58 @@ function resolveModulePackage() {
* @param {Object} options - the options to use
* @returns {Object} the discovered modules
*/
function discoverModules(options) {
function discoverModules(options) {
var pkg = options.pkg || packageUtils.getPackage(options.dir);
var cacheKey = JSON.stringify({
options: options,
pkg: pkg
});

// get the collection of dependencies
var dependencies = {};

// if there's package.json contents...
/* istanbul ignore else - defensive conditional, don't care about else-case */
if (pkg.data) {
merge(
dependencies,
// always include the `dependencies` and `peerDependencies`
pkg.data.dependencies,
pkg.data.peerDependencies,
// only include the `devDependencies` if isRoot
options.isRoot && pkg.data.devDependencies
);
}
return this.cache.modules.getOrElse(cacheKey, function() {
var dependencies = {};

// if there's package.json contents...
/* istanbul ignore else - defensive conditional, don't care about else-case */
if (pkg.data) {
merge(
dependencies,
// always include the `dependencies` and `peerDependencies`
pkg.data.dependencies,
pkg.data.peerDependencies,
// only include the `devDependencies` if isRoot
options.isRoot && pkg.data.devDependencies
);
}

// for each dependency...
dependencies = Object.keys(dependencies).reduce(function(modules, dependency) {
// resolve the package.json
var resolvedPkg = resolveModulePackage(
packageUtils.getPackagePath(dependency),
pkg.path,
URI.system(options.dir)
);
if (!resolvedPkg) {
// if it didn't resolve, they likely didn't `npm install` it correctly
this.issues.dependencies.missing.push(dependency);
} else {
// otherwise, set it onto our collection
var resolvedModule = resolveModule.call(this, resolvedPkg);
if (resolvedModule) {
modules[resolvedModule.name] = resolvedModule;
// for each dependency...
dependencies = Object.keys(dependencies).reduce(function(modules, dependency) {
// resolve the package.json
var resolvedPkg = resolveModulePackage(
packageUtils.getPackagePath(dependency),
pkg.path,
URI.system(options.dir)
);
if (!resolvedPkg) {
// if it didn't resolve, they likely didn't `npm install` it correctly
this.issues.dependencies.missing.push(dependency);
} else {
// otherwise, set it onto our collection
var resolvedModule = resolveModule.call(this, resolvedPkg);
if (resolvedModule) {
modules[resolvedModule.name] = resolvedModule;
}
}
}
return modules;
}.bind(this), {});
return modules;
}.bind(this), {});

// if it's the root...
if (options.isRoot) {
// ensure eyeglass itself is a direct dependency
dependencies.eyeglass = dependencies.eyeglass || getEyeglassSelf.call(this, options);
}
// if it's the root...
if (options.isRoot) {
// ensure eyeglass itself is a direct dependency
dependencies.eyeglass = dependencies.eyeglass || getEyeglassSelf.call(this, options);
}

return Object.keys(dependencies).length ? dependencies : null;
return Object.keys(dependencies).length ? dependencies : null;
}.bind(this));
}

module.exports = EyeglassModules;
6 changes: 4 additions & 2 deletions lib/modules/ModuleFunctions.js
Expand Up @@ -44,7 +44,8 @@ function ModuleFunctions(eyeglass, sass, options, existingFunctions) {
}

// log any functions found in this module
debug.functions(
/* istanbul ignore next - don't test debug */
debug.functions && debug.functions(
"functions discovered in module %s:%s%s",
mod.name,
DELIM,
Expand All @@ -60,7 +61,8 @@ function ModuleFunctions(eyeglass, sass, options, existingFunctions) {
functions = syncFn.all(functions);

// log all the functions we discovered
debug.functions(
/* istanbul ignore next - don't test debug */
debug.functions && debug.functions(
"all discovered functions:%s%s",
DELIM,
Object.keys(functions).join(DELIM)
Expand Down
64 changes: 64 additions & 0 deletions lib/util/SimpleCache.js
@@ -0,0 +1,64 @@
"use strict";

/**
* A simple caching implementation
*/
function SimpleCache() {
this.cache = {};
}

/**
* Returns the current cached value
*
* @param {String} key - they cache key to lookup
* @returns {*} the cached value
*/
SimpleCache.prototype.get = function(key) {
return this.cache[key];
};

/**
* Sets the cached value
*
* @param {String} key - they cache key to update
* @param {*} value - they value to store
*/
SimpleCache.prototype.set = function(key, value) {
this.cache[key] = value;
};

/**
* Whether or not the cache has a value for a given key
*
* @param {String} key - they cache key to lookup
* @returns {Boolean} whether or not the key is set
*/
SimpleCache.prototype.has = function(key) {
return this.cache.hasOwnProperty(key);
};

/**
* Gets the current value from the cache (if it exists), otherwise invokes the callback
*
* @param {String} key - they cache key lookup
* @param {Function} callback - the callback to be invoked when the key is not in the cache
* @returns
*/
SimpleCache.prototype.getOrElse = function(key, callback) {
// if we do not yet have a result, generate it and store it in the cache
if (!this.has(key)) {
this.set(key, callback());
}

// return the result from the cache
return this.get(key);
};

/**
* Purges the cache
*/
SimpleCache.prototype.purge = function(key) {
this.cache = {};
};

module.exports = SimpleCache;
6 changes: 5 additions & 1 deletion lib/util/debug.js
Expand Up @@ -4,6 +4,10 @@ var debug = require("debug");
var PREFIX = "eyeglass:";

module.exports = ["import", "modules", "functions", "assets"].reduce(function(obj, item) {
obj[item] = debug(PREFIX + item);
var namespace = PREFIX + item;
/* istanbul ignore if - don't test debug */
if (debug.enabled(namespace)) {
obj[item] = debug(namespace);
}
return obj;
}, {});