-
-
Notifications
You must be signed in to change notification settings - Fork 11
fix: Empty basePath should not be treated as cwd #120
base: main
Are you sure you want to change the base?
Conversation
In Node.js, `path.relative("")` always resolves to `process.cwd()`, which causes problems when people use the `Linter` API in ESLint where the `basePath` of a `ConfigArray` is set to an empty string. This ensures that only non-empty `basePath` strings are passed to `path.relative()`. Refs eslint/eslint#17669
tests/config-array.test.js
Outdated
it('should return a config when an absolute path filename matches a files pattern', () => { | ||
|
||
const config = configs.getConfig('/x/foo.js'); | ||
expect(config).to.deep.equal({ | ||
defs: { | ||
severity: 'error' | ||
} | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is expected behavior in this case:
configs = new ConfigArray([
{
files: ['x/*.js'],
defs: {
severity: 'error'
}
}
], { basePath: '', schema });
configs.normalizeSync();
const config = configs.getConfig('/x/foo.js'); // `undefined` or `{ defs: { severity: 'error' } }` ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting question. My initial reaction is that this should return undefined
, because the match is done from the left of the string, and because the pattern doesn't start with /
, that isn't a match. I've added some tests to this effect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would change how Linter works in browsers, where cwd
is /
(at least the way we bundle).
When I add the following to src/playground/index.js
:
import { Linter } from "./node_modules/eslint/lib/linter/";
const linter = new Linter({ configType: "flat" });
const code = "foo();";
const config = [{
files: ["bar/baz.js"],
rules: {
"no-undef": 2
}
}];
const filename = "/bar/baz.js";
const result = linter.verify(code, config, filename);
console.log(JSON.stringify(result, null, 4));
currently, it logs:
[
{
"ruleId": "no-undef",
"severity": 2,
"message": "'foo' is not defined.",
"line": 1,
"column": 1,
"nodeType": "Identifier",
"messageId": "undef",
"endLine": 1,
"endColumn": 4
}
]
because the config matches. However, after this change the config doesn't match and it logs:
[]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me, files: ['**/*.js']
matching '/x/foo.js'
is confusing, as file patterns are supposed to be relative to something. If we don't want basePath
to default to an absolute path (currently, it basically defaults to cwd
), then when basePath
isn't passed I'd rather expect this to work in "relative-only" mode and don't match any absolute paths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, note that after this change process.cwd()
would still be implicitly used when basePath
is a non-empty non-absolute path, which looks inconsistent with how empty basePath
is treated.
const configs = new ConfigArray([
{
files: ['**/*.js'],
defs: {
severity: 'error'
}
}
], { basePath: 'foo', schema }); // works the same as if it were `path.resolve('foo')`
configs.normalizeSync();
console.log(configs.getConfig('/foo/bar.js')); // undefined
console.log(configs.getConfig(path.resolve('foo/bar.js'))); // { defs: { severity: 'error' } }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me,
files: ['**/*.js']
matching'/x/foo.js'
is confusing, as file patterns are supposed to be relative to something. If we don't wantbasePath
to default to an absolute path (currently, it basically defaults tocwd
), then whenbasePath
isn't passed I'd rather expect this to work in "relative-only" mode and don't match any absolute paths.
I need some context for this: are you saying this is the way things work in ESLint currently or after the change in this PR?
In Node.js,
path.relative("")
always resolves toprocess.cwd()
, which causes problems when people use theLinter
API in ESLint where thebasePath
of aConfigArray
is set to an empty string.This ensures that only non-empty
basePath
strings are passed topath.relative()
.Refs eslint/eslint#17669
cc @mdjermanovic