Skip to content

feat: Support basePath property in config objects #131

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mdjermanovic
Copy link
Member

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

eslint/eslint#18948

@mdjermanovic mdjermanovic changed the title feat: Support basePath` property in config objects feat: Support basePath property in config objects Mar 31, 2025
@nzakas nzakas added the Initial Commenting This RFC is in the initial feedback stage label Mar 31, 2025

## Open Questions

Should this RFC include the `--basePath` feature (https://github.com/eslint/eslint/issues/19118)? I didn't see a relation between config-level base paths and ability to override config array's base path, so I didn't include it in this RFC.
Copy link
Member

Choose a reason for hiding this comment

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

The reason I thought it might be good to include this feature in this RFC is because of how --base-path might interact with a config basePath property. Would it just replace the config file base path completely such that any relative basePath property would now be relative to the value passed as --base-path?

Copy link
Member

Choose a reason for hiding this comment

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

I don't feel strongly about including the --base-path feature in this RFC but it seems useful to clarify how it will (or won't) interact with the basePath property.

@fasttime
Copy link
Member

fasttime commented Apr 2, 2025

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?

@nzakas
Copy link
Member

nzakas commented Apr 2, 2025

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?

@fasttime
Copy link
Member

fasttime commented Apr 3, 2025

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 mdjermanovic/rewrite#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.

@nzakas
Copy link
Member

nzakas commented Apr 8, 2025

Ah okay, so "parent config" is what I refer to as the "base config". 👍 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?

@mdjermanovic
Copy link
Member Author

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 #131 (comment), 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?

@nzakas
Copy link
Member

nzakas commented Apr 18, 2025

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
Copy link
Member

nzakas commented Apr 30, 2025

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

@mdjermanovic
Copy link
Member Author

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.

@nzakas
Copy link
Member

nzakas commented May 14, 2025

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
Copy link
Member

nzakas commented May 15, 2025

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 nzakas added the tsc agenda Topic for discussion at next TSC meeting label May 15, 2025

Currently, config array has a single property `basePath` (string) at the top level. When used from `eslint`, this property is set to the location of the config file, or the current working directory (when `--no-config-lookup` or `--config` options are used). `files` and `ignores` patterns in all config objects are treated as relative to the config array's `basePath`.

This RFC proposes allowing config objects to specify their own `basePath` property. When present, config object's `basePath` overrides config array's `basePath`, meaning that `files` and `ignores` patterns in that config object should be treated as relative to the config object's `basePath` instead of the config array's `basePath`. Also, when `basePath` is present in a config object that has neither `files` nor `ignores`, then the config object applies to files in `basePath` and its subdirectories only.
Copy link
Member Author

Choose a reason for hiding this comment

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

I've realized the behavior of basePath in configuration objects that have neither files nor ignores was not considered, so I updated this RFC in 09409ed, and the proof of concept accordingly in mdjermanovic/rewrite@d2ea0d7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Initial Commenting This RFC is in the initial feedback stage tsc agenda Topic for discussion at next TSC meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants