rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

feat: Support `basePath` property in config objects

Open mdjermanovic opened this issue 10 months ago • 2 comments

Summary

This RFC proposes supporting basePath property in config objects.

When present, config object's basePath overrides config array's basePath, meaning that files and ignores patterns in that config object will be treated as relative to the config object's basePath instead of the config array's basePath.

Related Issues

https://github.com/eslint/eslint/issues/18948

mdjermanovic avatar Mar 31 '25 12:03 mdjermanovic

I'm thinking about the situation where a parent config and an extended config have different base paths. Per the current defineConfig logic, the files and ignores properties would be merged, but the merged config will inherit the extended config's base path, which seems unexpected.

Maybe relative base paths in extended configs should be joined to the base path of the parent config?

fasttime avatar Apr 02 '25 10:04 fasttime

I'm thinking about the situation where a parent config and an extended config have different base paths.

I'm not sure I understand what you're describing. Can you provide an example?

nzakas avatar Apr 02 '25 18:04 nzakas

Here's an example where the basePath property is manually defined on the parent object:

export default defineConfig({
    basePath: "packages/foo",
    files: ["**/*.js"],
    rules: {
        rule1: "error"
    },
    extends: [{
        ignores: ["ignored.js"],
        rules: {
            rule2: "error"
        }
    }]
});

Using the fork in https://github.com/mdjermanovic/rewrite/pull/1, this will result in the following array:

[
    {
        ignores: ["ignored.js"],
        rules: { rule2: "error" },
        files: ["**/*.js"],
        name: "UserConfig[0] > ExtendedConfig[0]"
    },
    {
        basePath: "packages/foo",
        files: ["**/*.js"],
        rules: { rule1: "error" }
    }
]

Basically the merged config object (the first array element) now contains the files of the parent config without the basePath that should be used to resolve those patterns.

fasttime avatar Apr 03 '25 08:04 fasttime

Ah okay, so "parent config" is what I refer to as the "base config". :+1: I agree, it seems like when we extend a config, the result should contain the basePath from the parent/base config. @mdjermanovic what do you think?

nzakas avatar Apr 08 '25 18:04 nzakas

When the base config has basePath, and the extended config doesn't have basePath, I think the result (all produced config objects) should have basePath from the base config, because the base config restricts which files the extended config will be applied to, and in that respect the base config's basePath seems like a restriction (to a directory) that should apply the same way base config's file and ignores apply.

So, in the example from https://github.com/eslint/rfcs/pull/131#issuecomment-2774861203, I think the result should be:

[
    {
        basePath: "packages/foo",
        ignores: ["ignored.js"],
        rules: { rule2: "error" },
        files: ["**/*.js"],
        name: "UserConfig[0] > ExtendedConfig[0]"
    },
    {
        basePath: "packages/foo",
        files: ["**/*.js"],
        rules: { rule1: "error" }
    }
]

On the other hand, I'm not sure what to do when the extended config has basePath, as that might depend on the use cases for extends and basePath. A typical use case for extends is shareable/plugin configs, which I believe should not have basePath, so perhaps, to avoid confusion in expectations, defineConfig() should throw an error if it encounters basePath in extended configs?

mdjermanovic avatar Apr 17 '25 17:04 mdjermanovic

On the other hand, I'm not sure what to do when the extended config has basePath, as that might depend on the use cases for extends and basePath. A typical use case for extends is shareable/plugin configs, which I believe should not have basePath, so perhaps, to avoid confusion in expectations, defineConfig() should throw an error if it encounters basePath in extended configs?

That would be the first time defineConfig() restricts something that is allowed by ConfigArray, so I'm not in favor of that.

I'm not sure I agree that people won't use basePath in shared or plugin configs. I can certainly see that happening when people are using internal-only plugins or shareable configs.

So, I think it's useful to allow a config file to overwrite basePath in extended configs if necessary.

nzakas avatar Apr 18 '25 21:04 nzakas

@mdjermanovic @fasttime where do we want to go from here?

nzakas avatar Apr 30 '25 15:04 nzakas

So, I think it's useful to allow a config file to overwrite basePath in extended configs if necessary.

I'm not sure if overwriting basePath would be expected behavior.

For example, if this is a shared config:

export default [
    // apply this config everywhere
    {
        files: ["**/*.js"],
        rules: {
            "no-undef": 2,
        },
    },

    // apply this config only in `foo` directory
    {
        basePath: "foo",
        files: ["**/*.js"],
        rules: {
            "no-unused-vars": 2,
        },
    },
];

The intent is to define a general config for all files, and specific additions for a subdirectory.

Now, if this shared config is used with the intent to apply it to a particular directory only:

export default defineConfig([
    {
        basePath: "bar",
        extends: [sharedConfig],
    },
]);

Overwriting basePath would result in:

 [
    {
        basePath: "bar",
        files: ["**/*.js"],
        rules: {
            "no-undef": 2,
        },
    },

    {
        basePath: "bar",
        files: ["**/*.js"],
        rules: {
            "no-unused-vars": 2,
        },
    },
];

This means that both config objects apply to all files, which wasn't the original intent of the shared config.

mdjermanovic avatar May 14 '25 13:05 mdjermanovic

I guess I don't see that as a common case. In my mind, it's more common for either the shared config or the user config to have basePath, and rare that it would occur in both places. In other words, you either want to use a shared config that you know has basePath (probably because it's internal to your project), or you assume the shared config doesn't have basePath and you just want to limit where it's applied.

Given that the user should have ultimate control over where a config is applied, I still think overwriting basePath with the user config version makes sense.

nzakas avatar May 14 '25 15:05 nzakas

Because this issue of basePath in extends is the only thing left to resolve, let's go back to @mdjermanovic's original suggestion:

to avoid confusion in expectations, defineConfig() should throw an error if it encounters basePath in extended configs?

I'm okay with this, as it allows us the opportunity to think through this use case further and make changes down the line if necessary. I think it's important to get this change out relatively soon so we can start v10 planning.

@fasttime what do you think?

nzakas avatar May 15 '25 15:05 nzakas

We've agreed to throw an error when there's basePath in extends in May 15th TSC meeting.

sam3k avatar May 21 '25 00:05 sam3k

I've updated this RFC (https://github.com/eslint/rfcs/pull/131/commits/0997dc65a21579a5791d74dac2356bdfa51f049a) and the proof of concept (https://github.com/mdjermanovic/rewrite/pull/1/commits/8ee459a0badf46ecaacac21ed5655de4c245246b) with changes in defineConfig().

mdjermanovic avatar May 21 '25 14:05 mdjermanovic

This is now ready for re-review.

mdjermanovic avatar May 21 '25 14:05 mdjermanovic