Skip to content
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.

Sign up for GitHub

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

Jump to bottom

New: Changing the Default Value of --resolve-plugins-relative-to #39

Closed
wants to merge 4 commits into from
Closed

New: Changing the Default Value of --resolve-plugins-relative-to #39

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
259b7d0
New: Changing the Default Value of --resolve-plugins-relative-to
mysticatea Sep 28, 2019
dbb0c1b
add PR link
mysticatea Sep 28, 2019
e9a2cd1
Update designs/2019-changing-default-of-resolve-plugins-relative-to/R…
mysticatea Oct 24, 2019
f6420a0
separate this RFC
mysticatea Nov 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
94 changes: 94 additions & 0 deletions designs/2019-changing-default-of-resolve-plugins-relative-to/README.md
View file
Open in desktop
@@ -0,0 +1,94 @@
- Start Date: 2019-09-28
- RFC PR: https://github.com/eslint/rfcs/pull/39
- Authors: Toru Nagashima ([@mysticatea](https://github.com/mysticatea))

# Changing the Default Value of `--resolve-plugins-relative-to`

## Summary

This RFC changes the default value of `--resolve-plugins-relative-to` CLI option to work on more variety environments.

## Motivation

We can use the config file which is in a common directory by `--config` option. In that case, it's intuitive to load plugins from the directory that contains the config file. But currently, we have to use `--resolve-plugins-relative-to` option along with `--config` option, or have to install required plugins into the current working directory. For example:

```
eslint . --config /home/me/.eslintrc.js --resolve-plugins-relative-to /home/me
```

```
eslint projects/foo --config projects/shared/.eslintrc.js --resolve-plugins-relative-to projects/shared
```

This behavior is not convenient.

## Detailed Design

### `--resolve-plugins-relative-to` CLI option

This RFC changes the default value of `--resolve-plugins-relative-to` CLI option.

1. If `--resolve-plugins-relative-to` CLI option is present, adopt it.
1. If `--config` CLI option is present, adopt the directory that contains the config file.
1. Otherwise, adopt the current working directory.

This setting is used in the whole eslint process. Therefore, every rule ID is identified as unique, so the `rulesMeta` of formatters works fine.

### `resolvePluginsRelativeTo` constructor option of CLIEngine

This RFC changes the default value of `resolvePluginsRelativeTo` option.

1. If `resolvePluginsRelativeTo` constructor option is present, adopt it.
1. If `configFile` constructor option is present, adopt the directory that contains the config file.
1. Otherwise, adopt the current working directory.

This setting is used in the whole `CLIEngine` instance. Therefore, every rule ID is identified as unique, so the `CLIEngine#getRules()` method works fine.

### About personal config file (`~/.eslintrc`)

This RFC doesn't change the plugin base path for the personal config file (`~/.eslintrc`) because the functionality has been deprecated in [RFC 32](https://github.com/eslint/rfcs/pull/32). ESLint still loads plugins from the current working directory if there is no project config file, even if there is the personal config file.

If people want to use the personal config file, use `--config` option (e.g., `eslint . --config ~/.eslintrc.json`). It should work intuitively.
mysticatea marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +47 to +51
Copy link

Choose a reason for hiding this comment

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

This feels so unproductive. I'm basically saying: This is poor wording, and you keep telling me how the software works.

### About personal config file (`~/.eslintrc`)

This RFC is essentially unrelated to the handling of personal config files. In particular, we do 
not detect if the user provides `--config ~/.eslintrc` and handle it any different. Therefore, with 
this in effect, we would set `resolvePluginsRelativeTo` to the HOME dir in such cases. This can 
confuse users which might expect we handle a personal config file differently i.e. by instead 
resolving globally installed packages. It could also encourage users to install into the HOME dir. 
However, we clearly do not recommend users install anything into `~/node_modules`.

Something like that. So I basically change "intuitively" to "confuse".

Does that help?


### Special handling of `~/`
Copy link

Choose a reason for hiding this comment

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

Also, just to make it clear (and, yes, I know that I'm coming from nowhere here, and leave comments in a repo I may not belong to). IMO (and as a Windows user) a) this is YAGNI and probably has more maintenance cost as the core change, b) this is unrelated and should get its own RFC.


Bash replaces `~` by the user home directory, but this behavior is not compatible with other platforms. This is similar to that we cannot use glob patterns on every platform.

Therefore, this RFC adds handling of `~`. In the following constructor options, if the path was exactly `~` or started with `~/` (and `~\` as well on Windows), the `CLIEngine` class replaces the `~` by `require("os").homedir()`.

- `cacheLocation`
- `configFile`
- `cwd`
- `ignorePath`
- `resolvePluginsRelativeTo`

Notes:

- It doesn't handle `~username/` notation because Node.js doesn't provide the way that gets the home directory of other users.
- It doesn't handle `~` in config files because this is a fallback of stuff that some shells do.

## Documentation

- It needs an entry in the migration guide because of a breaking change.
- It updates the "[Command Line Interface](https://eslint.org/docs/user-guide/command-line-interface)" page to describe the `--config` option changes the default value of the `--resolve-plugins-relative-to` option.
- It updates the "[Command Line Interface](https://eslint.org/docs/user-guide/command-line-interface)" page to describe the `--config` option changes the default value of the `--resolve-plugins-relative-to` option.
- It updates the "[Node.js API](https://eslint.org/docs/developer-guide/nodejs-api)" page to describe the `configFile` constructor option changes the default value of the `resolvePluginsRelativeTo` constructor option on CLIEngine.

## Drawbacks

- The new logic is complex a bit. It increases maintenance cost.
Copy link

Choose a reason for hiding this comment

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

Add

  • Given a personal config file, user might feel encouraged to install packages into the HOME dir. See discussion above.

Probably remove "The new logic is complex a bit" because it is not.


## Backwards Compatibility Analysis

This RFC is a breaking change but has a small impact.

If people use `--config` option with the path to outside of the current working directory and don't use `--resolve-plugins-relative-to` option, and the referred config depends on the plugins of the current working directory, then this RFC breaks the workflow. (I guess it's a rare case)

## Alternatives

Nothing in particular.

## Related Discussions

- https://github.com/eslint/eslint/pull/11696 - New: add `--resolve-plugins-relative-to` flag
- https://github.com/eslint/eslint/issues/11914 - Personal config (`~/.eslintrc`) doesn't load global-installed configs/parsers/plugins