Permalink
Browse files

Fix: ensure configs from a plugin are cached separately (fixes #8792) (

…#8798)

* Fix: ensure configs from a plugin are cached separately (fixes #8792)

Config files are cached by file path to avoid needing to load the same config file twice. However, configs provided by plugins all have the same file path (the index file of the plugin). This created a bug when loading multiple configs for the same plugin where only the highest-precedence config from that plugin would be loaded. All other configs from that plugin would be considered identical by the cache, so they would all end up getting pulled from the cache as the same config.

This commit updates the caching logic to use the config full name as a cache index. This is still the file path when resolving a config from the filesystem, but it is the unique plugin config identifier (e.g. 'plugin:foo/node-config') when resolving a plugin config.

* Remove unused second argument from loadConfigFile call

* Fix inaccurate comments for config-cache methods
  • Loading branch information...
not-an-aardvark authored and ilyavolodin committed Jun 26, 2017
1 parent 8b48ae8 commit f307aa0bd4a21bd9e6147a067b30564f85c9a4df
View
@@ -29,31 +29,33 @@ function hash(vector) {
module.exports = class ConfigCache {
constructor() {
this.filePathCache = new Map();
this.configFullNameCache = new Map();
this.localHierarchyCache = new Map();
this.mergedVectorCache = new Map();
this.mergedCache = new Map();
}
/**
* Gets a config object from the cache for the specified config file path.
* @param {string} configFilePath the absolute path to the config file
* @param {string} configFullName the name of the configuration as used in the eslint config(e.g. 'plugin:node/recommended'),
* or the absolute path to a config file. This should uniquely identify a config.
* @returns {Object|null} config object, if found in the cache, otherwise null
* @private
*/
getConfig(configFilePath) {
return this.filePathCache.get(configFilePath);
getConfig(configFullName) {
return this.configFullNameCache.get(configFullName);
}
/**
* Sets a config object in the cache for the specified config file path.
* @param {string} configFilePath the absolute path to the config file
* @param {string} configFullName the name of the configuration as used in the eslint config(e.g. 'plugin:node/recommended'),
* or the absolute path to a config file. This should uniquely identify a config.
* @param {Object} config the config object to add to the cache
* @returns {void}
* @private
*/
setConfig(configFilePath, config) {
this.filePathCache.set(configFilePath, config);
setConfig(configFullName, config) {
this.configFullNameCache.set(configFullName, config);
}
/**
View
@@ -488,12 +488,15 @@ function normalizePackageName(name, prefix) {
* @returns {Object} An object containing 3 properties:
* - 'filePath' (required) the resolved path that can be used directly to load the configuration.
* - 'configName' the name of the configuration inside the plugin.
* - 'configFullName' the name of the configuration as used in the eslint config (e.g. 'plugin:node/recommended').
* - 'configFullName' (required) the name of the configuration as used in the eslint config(e.g. 'plugin:node/recommended'),
* or the absolute path to a config file. This should uniquely identify a config.
* @private
*/
function resolve(filePath, relativeTo) {
if (isFilePath(filePath)) {
return { filePath: path.resolve(relativeTo || "", filePath) };
const fullPath = path.resolve(relativeTo || "", filePath);
return { filePath: fullPath, configFullName: fullPath };
}
let normalizedPackageName;
@@ -510,7 +513,7 @@ function resolve(filePath, relativeTo) {
normalizedPackageName = normalizePackageName(filePath, "eslint-config");
debug(`Attempting to resolve ${normalizedPackageName}`);
filePath = resolver.resolve(normalizedPackageName, getLookupPath(relativeTo));
return { filePath };
return { filePath, configFullName: filePath };
}
@@ -580,7 +583,7 @@ function loadObject(configObject, configContext) {
function load(filePath, configContext, relativeTo) {
const resolvedPath = resolve(filePath, relativeTo);
const cachedConfig = configContext.configCache.getConfig(resolvedPath.filePath);
const cachedConfig = configContext.configCache.getConfig(resolvedPath.configFullName);
if (cachedConfig) {
return cachedConfig;
@@ -591,7 +594,7 @@ function load(filePath, configContext, relativeTo) {
if (config) {
config.filePath = resolvedPath.filePath;
config.baseDirectory = path.dirname(resolvedPath.filePath);
configContext.configCache.setConfig(resolvedPath.filePath, config);
configContext.configCache.setConfig(resolvedPath.configFullName, config);
}
return config;
@@ -0,0 +1,3 @@
extends:
- plugin:test/foo
- plugin:test/bar
@@ -899,6 +899,49 @@ describe("ConfigFile", () => {
}
});
});
it("should load two separate configs from a plugin", () => {
const stubConfig = new Config({}, new Linter());
const resolvedPath = path.resolve(PROJECT_PATH, "./node_modules/eslint-plugin-test/index.js");
const configDeps = {
"../util/module-resolver": createStubModuleResolver({
"eslint-plugin-test": resolvedPath
}),
"require-uncached"(filename) {
return configDeps[filename];
}
};
configDeps[resolvedPath] = {
configs: {
foo: { rules: { semi: 2, quotes: 1 } },
bar: { rules: { quotes: 2, yoda: 2 } }
}
};
const StubbedConfigFile = proxyquire("../../../lib/config/config-file", configDeps);
const configFilePath = getFixturePath("plugins/.eslintrc2.yml");
const config = StubbedConfigFile.load(configFilePath, stubConfig);
assert.deepEqual(config, {
baseDirectory: path.dirname(configFilePath),
filePath: configFilePath,
parserOptions: {},
globals: {},
env: {},
rules: {
semi: 2,
quotes: 2,
yoda: 2
},
extends: [
"plugin:test/foo",
"plugin:test/bar"
]
});
});
});
describe("even if config files have Unicode BOM,", () => {

0 comments on commit f307aa0

Please sign in to comment.