Skip to content
This repository has been archived by the owner on Jun 10, 2024. It is now read-only.

Commit

Permalink
fix: Only apply universal patterns if others match.
Browse files Browse the repository at this point in the history 
Previously, patterns such as /** and /* would always match and apply
those configs to the result. In reality, these patterns should
effectively work the same as configs without a 'files' entry: they
should only apply if there is another matching config.
  • Loading branch information
@nzakas
nzakas committed Oct 21, 2022
1 parent 549f13b commit e69c8fd
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 6 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ A few things to keep in mind:
* You must pass in the absolute filename to get a config for.
* The returned config object never has `files`, `ignores`, or `name` properties; the only properties on the object will be the other configuration options specified.
* The config array caches configs, so subsequent calls to `getConfig()` with the same filename will return in a fast lookup rather than another calculation.
* A config will only be generated if the filename matches an entry in a `files` key. A config will not be generated without matching a `files` key (configs without a `files` key are only applied when another config with a `files` key is applied; configs without `files` are never applied on their own).
* A config will only be generated if the filename matches an entry in a `files` key. A config will not be generated without matching a `files` key (configs without a `files` key are only applied when another config with a `files` key is applied; configs without `files` are never applied on their own). Any config with a `files` key entry ending with `/**` or `/*` will only be applied if another entry in the same `files` key matches or another config matches.

## Determining Ignored Paths

Expand Down
73 changes: 69 additions & 4 deletions src/config-array.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ function isString(value) {
return typeof value === 'string';
}

/**
* Asserts that the files key of a config object is a nonempty array.
* @param {object} config The config object to check.
* @returns {void}
* @throws {TypeError} If the files key isn't a nonempty array.
*/
function assertNonEmptyFilesArray(config) {
if (!Array.isArray(config.files) || config.files.length === 0) {
throw new TypeError('The files key must be a non-empty array.');
}
}

/**
* Wrapper around minimatch that caches minimatch patterns for
* faster matching speed over multiple file path evaluations.
Expand Down Expand Up @@ -233,9 +245,7 @@ function pathMatches(filePath, basePath, config) {
const relativeFilePath = path.relative(basePath, filePath);

// if files isn't an array, throw an error
if (!Array.isArray(config.files) || config.files.length === 0) {
throw new TypeError('The files key must be a non-empty array.');
}
assertNonEmptyFilesArray(config);

// match both strings and functions
const match = pattern => {
Expand Down Expand Up @@ -690,15 +700,70 @@ export class ConfigArray extends Array {

const matchingConfigIndices = [];
let matchFound = false;
const universalPattern = /\/\*{1,2}$/;

this.forEach((config, index) => {

if (!config.files) {
debug(`Universal config found for ${filePath}`);
debug(`Anonymous universal config found for ${filePath}`);
matchingConfigIndices.push(index);
return;
}

assertNonEmptyFilesArray(config);

/*
* If a config has a files pattern ending in /** or /*, and the
* filePath only matches those patterns, then the config is only
* applied if there is another config where the filePath matches
* a file with a specific extensions such as *.js.
*/

const universalFiles = config.files.filter(
pattern => universalPattern.test(pattern)
);

// universal patterns were found so we need to check the config twice
if (universalFiles.length) {

debug('Universal files patterns found. Checking carefully.');

const nonUniversalFiles = config.files.filter(
pattern => !universalPattern.test(pattern)
);

// check that the config matches without the non-universal files first
if (
nonUniversalFiles.length &&
pathMatches(
filePath, this.basePath,
{ files: nonUniversalFiles, ignores: config.ignores }
)
) {
debug(`Matching config found for ${filePath}`);
matchingConfigIndices.push(index);
matchFound = true;
return;
}

// if there wasn't a match then check if it matches with universal files
if (
universalFiles.length &&
pathMatches(
filePath, this.basePath,
{ files: universalFiles, ignores: config.ignores }
)
) {
debug(`Matching config found for ${filePath}`);
matchingConfigIndices.push(index);
return;
}

// if we make here, then there was no match
return;
}

// the normal case
if (pathMatches(filePath, this.basePath, config)) {
debug(`Matching config found for ${filePath}`);
matchingConfigIndices.push(index);
Expand Down
43 changes: 42 additions & 1 deletion tests/config-array.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ function createConfigArray(options) {
},
{
ignores: [filePath => filePath.endsWith('.gitignore')]
},
{
files: ['**/*'],
defs: {
universal: true
}
}
], {
basePath,
Expand Down Expand Up @@ -497,6 +503,7 @@ describe('ConfigArray', () => {
expect(config.language).to.equal(JSLanguage);
expect(config.defs).to.be.an('object');
expect(config.defs.name).to.equal('config-array');
expect(config.defs.universal).to.be.true;
expect(config.defs.css).to.be.false;
});

Expand All @@ -507,6 +514,7 @@ describe('ConfigArray', () => {

expect(config.defs).to.be.an('object');
expect(config.defs.name).to.equal('config-array');
expect(config.defs.universal).to.be.true;
expect(config.defs.xyz).to.be.true;
});

Expand All @@ -517,6 +525,7 @@ describe('ConfigArray', () => {

expect(config.defs).to.be.an('object');
expect(config.defs.name).to.equal('HTML');
expect(config.defs.universal).to.be.true;
});

it('should return undefined when passed ignored .gitignore filename', () => {
Expand All @@ -536,6 +545,7 @@ describe('ConfigArray', () => {
expect(config.defs).to.be.an('object');
expect(config.defs.name).to.equal('config-array.test');
expect(config.defs.css).to.be.false;
expect(config.defs.universal).to.be.true;
});

it('should calculate correct config when passed JS filename that matches a function config', () => {
Expand All @@ -547,6 +557,7 @@ describe('ConfigArray', () => {
expect(config.defs).to.be.an('object');
expect(config.defs.name).to.equal('from-context');
expect(config.defs.css).to.be.false;
expect(config.defs.universal).to.be.true;
});

it('should not match a filename that doesn\'t explicitly match a files pattern', () => {
Expand Down Expand Up @@ -612,6 +623,7 @@ describe('ConfigArray', () => {
expect(config.defs).to.be.an('object');
expect(config.defs.name).to.equal('async-from-context');
expect(config.defs.css).to.be.false;
expect(config.defs.universal).to.be.true;
});

it('should calculate correct config when passed JS filename that matches a function config returning an array', () => {
Expand All @@ -638,7 +650,7 @@ describe('ConfigArray', () => {
expect(config.language).to.equal(CSSLanguage);
expect(config.defs).to.be.an('object');
expect(config.defs.name).to.equal('config-array');

expect(config.defs.universal).to.be.true;
});

it('should calculate correct config when passed JS filename that matches AND pattern', () => {
Expand All @@ -649,6 +661,7 @@ describe('ConfigArray', () => {
expect(config.defs).to.be.an('object');
expect(config.defs.name).to.equal('AND operator');
expect(config.defs.css).to.be.false;
expect(config.defs.universal).to.be.true;
});

it('should return the same config when called with the same filename twice (caching)', () => {
Expand Down Expand Up @@ -1042,6 +1055,34 @@ describe('ConfigArray', () => {
expect(configs.isFileIgnored(path.join(basePath, 'node_modules/package/a.js'))).to.be.false;
});

it('should return true when there are only patterns ending with /*', () => {
configs = new ConfigArray([
{
files: ['foo/*']
}
], {
basePath
});

configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo/a.js'))).to.be.true;
});

it('should return true when there are only patterns ending with /**', () => {
configs = new ConfigArray([
{
files: ['foo/**']
}
], {
basePath
});

configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo/a.js'))).to.be.true;
});

});

describe('isDirectoryIgnored()', () => {
Expand Down

0 comments on commit e69c8fd

Please sign in to comment.