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.

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

Need to always use global 'eslint' #270

Open
getify opened this Issue Mar 2, 2019 · 18 comments

Comments

3 participants
@getify
Copy link

getify commented Mar 2, 2019

Is there a way to configure this plugin so that it always uses a globally installed 'eslint'?

I'm fine with it picking up local eslint configurations, but it's really troublesome that it picks up on locally installed eslint instances. That creates potential havoc where you think some version of eslint is running, but there happens to be a different version installed locally that you may not even know about. :/


The real problem I have is, I have written an eslint rule plugin that I have installed globally. I'm able to get my global eslint to load and use it just fine, even when running inside of Sublime. But if I happen to be in a directory where there's a local eslint installed, SublimeLinter-eslint chooses the local eslint, and that local eslint cannot see any globally installed eslint rule-plugins.

Moreover, since I'm the author of the eslint-rule plugin, when I distribute it to others, I would normally tell them to install it once, globally. But it's a huge footgun for my users that it may all of a sudden be broken if they switch into a directory on their machine where eslint happens to be locally installed.

@kaste

This comment has been minimized.

Copy link
Contributor

kaste commented Mar 4, 2019

I read that issue three times, and I really think it is an odd one. Is this essentially for dev beginners?

Usually local configuration can depend on local (pinned) eslint installations, potentially including pinned plugins. Since a green run of eslint is a gatekeeper for PR's or any other contribution to a project it is really recommended to use these exact versions. If instead the project doesn't depend on a green eslint, it probably does not and should not install eslint locally and each dev can choose to run a linter or not.

Is your plugin a super-magic plugin so you cannot recommend to install and add it as a dependency locally? Bc maybe it helps beginners with tips and tricks but teams cannot reach consensus to include it?

As it stands this is not implemented and I don't know a workaround. Since we inherit the lookup behavior from SublimeLinter core (namely 'NodeLinter' over there) this must be implemented in core.

But again say we have a 'always_use_global_installation'. Would you recommend it to set this globally to 'true' (and then everyone is confused if a project really needs the local installation). Or do you expect people set this 'per project'. If so they already configure/twiddle and probably could also tell npm to uninstall eslint locally.

@getify

This comment has been minimized.

Copy link
Author

getify commented Mar 4, 2019

Let me re-emphasize that I entirely understand the benefits of having per-project local configuration. If I have multiple projects, each with their own linting opinions, I would be greatly pleased that ESLint and Sublime-eslint were picking up and using the appropriate rules per project.

What I don't understand, and what is frustrating my use-case, is how Sublime-eslint is picking up on a per-project install of ESLint, but in my case using still a global configuration. That seems crazy and seems like it would always be a terrible idea.

I would think the behavior should be: if you find a local config, use it. If you find a local config and local install, use both (they're almost certainly compatible). But if you find only a local install without a local config, do not use the local install, and instead fall back to finding and using the global config and install. Never try to use a local install with a global config, because there's a strong likelihood of incompatibility.

But even if you didn't want to bake that behavior in as default, what I'm asking is, for a configuration, that turns that behavior on, that I can opt into.

Again, to state clearly: I only want the Sublime-eslint tool to use a local ESLint install if it can also find a local configuration.

@getify

This comment has been minimized.

Copy link
Author

getify commented Mar 4, 2019

To provide further clarity on what my use-case problem is, let me try to explain this problem with a different kind of scenario, to see if it's more clear.

I have a global install of ESLint (v5.15) on my system. I also have a "global" (that is, in my home directory) eslint config. I also have a variety of ESLint plugins (custom rules) installed globally.

I manage 15 different open source projects, and for all those, I use exactly the same coding style, so that one set of global ESLint config is good enough for all those projects. I don't need a per-project config, nor do I need per-project installation of those custom rules plugins.

Then one day, I run across a cool github project called "XYZ" that I want to help with. I fork/clone that project to my laptop, run an npm install on it to get any dependencies I need, and I open it up in my Sublime. I notice immediately that I'm getting linter failures in my Sublime console. Those errors don't show up when I work on any of my other projects, so I'm confused.

This "XYZ" project I've downloaded, it doesn't really seem to have anything to do with ESLint, at least from what I can tell, so I don't know why ESLint linting would somehow be broken in only that project. But I look in its node_modules/ directory, and sure enough, there's an installed version of ESLint there locally.

As I come to find out, that local copy of ESLint is in there from one of the "XYZ" project's "dev dependencies" (installed b/c of npm install). However, it's used (via the Node.js API of ESLint) in a part of the project that I'm not particularly interested in. In fact, they're not really using ESLint for linting at all -- just some other purpose, who knows what? Anyway, what's worse, I realize that this local copy of ESLint is version 3.0, which is way behind, and incompatible with, my global ESLint (5.15).

Further, I discover that they don't ship an .eslintrc.json in the "XYZ" package because, again, they don't really use ESLint as a CLI tool, they only have the dev-dependency so they can do API stuff with it, and they have no need for config files.

So... now I fully understand the problem. An old ESLint v3 is running locally, and thus running in my Sublime editor. But it's picking up on my global .eslintrc.json config file. That config file of course assumes as ESLint of >= 5.x. Also, that config file tries to load globally installed ESLint rule plugins, some of which may not even be compatible with that old ESLint 3.0.

Here's my conundrum. I had absolutely no knowledge or control over the fact that this incompatible old ESLint was going to be installed locally, nor that it was going to come without a valid local config file for the project. But simply because it exists within this "XYZ" project locally, now I can't really use my editor with ESLint linting AT ALL inside the "XYZ" project. This hurts my productivity as a developer greatly.

What are my solutions? Well...

  1. I could remove the offending ESLint install from the local node_modules/, and hope that doesn't break something about the project that I am doing now or in the future.

  2. I could try to make a copy of my .eslintrc.json config file and place it locally, and tinker around trying to get something that will not break that v3.0.

  3. I could try to add local installs of my globally installed rule plugins, and hope none of them break 3.0 (they probably will).

All of these are terrible options, and cost me a lot in productivity.

They leave me wishing there was some way to tell Sublime-eslint, "Hey, even though you see this locally installed ESLint, since there's no local config, please just ignore that install and keep on doing ESLint linting the way I've always done."

I hope that motivation makes more sense.

@getify

This comment has been minimized.

Copy link
Author

getify commented Mar 4, 2019

To provide one further point of clarity... here's how I actually discovered this problem.

I was literally building an ESLint rule plugin. Of course, to build and test ESLint plugins, you have to have a local install of ESLint (it's a "peer dependency"), which in my case I was using not for the CLI but using for the Node.js API to run my test suite for my plugin. So obviously I had no local .eslintrc.json, because there's no need for one in that situation.

But I noticed that while working on my plugin, my entire ESLint linting experience in Sublime was dead (no linting at all).

Upon investigation, I found out that it's because my global eslint config (which has my plugin loaded/enabled) was trying to find my plugin, but since Sublime-eslint was using my local ESLint, it was trying to find the plugin INSIDE the plugin.

Obviously, the plugin can't be installed inside itself. That doesn't even make any sense.

Literally, to fix this bug, and still be able to lint myself while working on my ESLint rule plugin, I had to go into my local node_modules/ and do a circular Symlink of my plugin's name pointing back to the main plugin directory itself.

That's crazy, right?


So here's what I am imagining... anyone who uses Sublime-eslint and who sometime decides to download my plugin package to try to contribute... also their linting will be broken.

And what am I going to have to explain to those confused contributers? This convoluted nonsense of, "go do a circular symlink", or "make a fake .eslintrc.json locally", or...

These are all bad options that will just end up with people being confused.

It would be immensely easier to just document in the project FAQ: "if you use Sublime-eslint while working on this project, please flip this configuration preference on".

@kaste

This comment has been minimized.

Copy link
Contributor

kaste commented Mar 5, 2019

🤔 A bit more focused and less wordy would increase the chance that I don't miss essential information here. 😁

Now, to lookup up and match a config file with its installation sounds good. But we don't do that at all right now. Basically for all linters the config file lookup is convoluted and eslint ... well it has three file formats or a special key in package.json. Don't even know if the leading '.' in '.eslintrc...' is mandatory. Should we just match a config file right beside a 'package.json'? It's easy to get this wrong, and if you get it right it's easy to get outdated.

Can you think of a different heuristic only based on 'package.json'? Because we lookup that file anyway, and also read out the "bin" -> "eslint" value if present.

So SublimeLinter actually only cares about finding an 'executable'. For this plugin we automatically prefer a local installation. A user can override manually by defining executable in the settings. (Usually you tell a user to exec which/where eslint on the command line and to paste that value.)

I'm usually mildly 👍 if we can do something automatic. But if the user already is confused and becomes annoyed because it doesn't work out of the box, we enter manual, config-hell mode, and then it is not necessarily easier to tell the user to flip a flag but to empower. Running 'which' and putting that value into 'executable' which is a valid setting across all SublimeLinter plugins is more useful knowledge than "set 'only_use_global_eslint' to 'true'".

But again, if there are simple algorithms to help the user diagnose or auto-solve the issue here 👍 unfortunately rants are not specs and automatic is not automagic.

@mohitsinghs

This comment has been minimized.

Copy link

mohitsinghs commented Mar 5, 2019

@kaste Some of us prefer to use global eslint if most of our projects are using same config, but it breaks with this plugin because it prefers local eslint by default. The problem with this approach is —

  • If we try to set global executable then projects having local config with local eslint will use global eslint. That's what I am facing.
  • If we do nothing then even if local eslint config is not found and local eslint exist for some reason ( peerDependency for example ), that local eslint will be used with global config . That's what @getify is facing as far I can understand.

There might be some other cases I missed.

The apparent solution is to use local eslint only if local config exists and global otherwise. Once we decide between local and global executable, the rest will be easily handled by eslint itself.

If, for some reason, this can't be achieved with this plugin, another solution I might give a try is to create a cli wrapper around eslint which will decide between global and local eslint based on existence of local config and expose that as is and then pass that as executable.

Again, this might be an opt-in config for this plugin. ( Like use_local_if_config_exists ). Sorry, I'm not good at naming but the point is to have an option to use local eslint only if local config is found vs prefer local eslint because local eslint might exist for other reasons.

Also, eslint config files formats are limited and less likely to break but I admit they are tricky to track.

That's all I can say based on my limited understanding. I hope this helps.

@getify

This comment has been minimized.

Copy link
Author

getify commented Mar 5, 2019

@kaste

Rather than letting ESLint pick its own location for configuration, it seems like the "best" path here is to develop some reasonable lookup heuristic-algorithm for ESLint configs, and then when invoking ESLint (global or local) forcibly tell it to use only that specifically identified config.

That seems like a fairly "bullet proof" way of making sure the heuristic/algorithm used doesn't end up disagreeing with ESLint; just remove eslint's decision making from the process, and substitute our own. That's more work, but it's a better outcome.

And you can just document this lookup behavior explicitly, so users of Sublime-eslint at least can understand it (where, arguably, understanding ESLint's current behavior is rather tricky).

As for the package.json... I would say if you find an eslintConfig in it, pull it out, parse it, and pass those options to ESLint as CLI params. I think you'd also need to pass a CLI param pointing to an "empty" configuration file, so as to disable ESLint trying to find its own config, thereby only getting configs from the passed in params.


So... a suggested heuristic/algorithm to consider and discuss:

  1. Start with the current directory of the active file.

  2. If there's no identified config yet:

    • Look in the current directory of the file, for a .eslintrc.json file. If so, save this config location.

    • If not, try to find a package.json that has a eslintConf key in it. If so, pull out that config.

    • if config is not yet found, JUMP to (7).

  3. Determine if we're still in the same directory as that found config. If not, JUMP to (5).

  4. Look to see if there's a node_modules/ subdirectory with an ESLint installed inside it. If so, save this executable location.

  5. If there's a config and executable location identified, JUMP to (9).

  6. If we're at the user's home directory, JUMP to (9).

  7. If we're at the top of the filesystem, and haven't yet visited the user's home directory, try the user's home directory, JUMP to (2).

  8. move up one directory, JUMP to (2).

  9. If an executable has not yet been identified, try to find the global ESLint. If not found, STOP.

  10. Invoke the identified executable with the identified config (if any).


It's a separate question of whether the proposed behavior here would be default/fixed in Sublime-eslint, or just behind a pref-flag. I think it makes more sense as the former, but I can understand why it might be safer to leave it as an option...

Maybe a compromise is to announce in the next release this new behavior behind an option, with a note that in a future release it may become deprecated, and eventually the default behavior, with the option going away.

@mohitsinghs

This comment has been minimized.

Copy link

mohitsinghs commented Mar 6, 2019

@getify Thanks for the suggestion, I tired doing a fragile implemention based on that and got it working in my limited test by monkey-patching linter.py locally. It's not exact and can be optimized but it's using local eslint only if local config is found and that config is passed to eslint executable in that case.

@kaste Sorry, if I did some silly mistakes as It was the limit of my basic python understanding 😂.

@kaste

This comment has been minimized.

Copy link
Contributor

kaste commented Mar 7, 2019

🤔 It's really not the domain of this plugin or SublimeLinter to look up config files and then to pass it down to the cli invocation. That's just a recipe for unwanted complexity. And I'm somewhat confused that you, @getify, propose such a way.

Now first, what would be the reason to tell eslint which config it should use?

There is also a thin line here as follows: SublimeLinters business is to find an executable, and it can use reasonable information to choose a good one. To get that information I can imagine we look around if we see a 'eslint' config file along our way. But that doesn't mean we tell eslint to use a specific config, it just means we prefer a specific eslint instance.

There are also a lot of thin lines between a heuristic (which I asked for) and pseudo-speccy goto bullet lists. As far as I understand, the problem arises from projects having eslint checked out/installed but not for cli usage but merely as a code dependency. So I think it is reasonable to ask first can we infer by observation, maybe looking into package.json, if the installed eslint instance is good for us to use.

@mohitsinghs Always good to see code but yours doesn't walk up the dir hierarchy. It just goes to the parent. But even for shitty code you can open PRs. 😁 That's still easier to checkout and to comment. Don't be fooled by the base NodeLinter. I never had the time to update it, but if we touch it now we really wipe the old implementation away.

Here are some plumbing vehicles:

def paths_upwards(path):
    # type: (str) -> Iterator[str]
    while True:
        yield path

        next_path = os.path.dirname(path)
        if next_path == path:
            return

        path = next_path

I think we only need to go up to HOME though

HOME = os.path.expanduser('~')
def paths_upwards_until_home(path):
    # type: (str) -> Iterator[str]
    return takewhile(lambda p: p != HOME, paths_upwards(path))

This can be used like so

def find_local_linter_installation(path, npm_name='eslint'):
    # type: (str, str) -> Optional[str]
    for path in paths_upwards(path):  # or *until_home()
        node_modules_bin = os.path.join(path, 'node_modules', '.bin')
        executable = shutil.which(npm_name, path=node_modules_bin)
        if executable:
            return executable
    else: 
        return None

For reading 'package.json' I would like to see something like this if possible:

def read_json_file(path):
    # type: (str) -> Dict[str, Any]
    return _read_json_file(path, os.path.getmtime(path))


@lru_cache(maxsize=1)
def _read_json_file(path, _mtime):
    # type: (str, float) -> Dict[str, Any]
    with open(path, 'r', encoding='utf8') as f:
        return json.load(f)

I've read into the eslint code, and it either uses dirname(filename) if filename provided or cwd as starting dir. cwd is configurable by the user. So we need to stick to

start_dir = (
	self.settings.get('file_path') 
	or self.get_working_dir(self.settings)
    or os.path.realpath('.')
)

This is all untested code, and I don't know yet how these bits fit together.

@mohitsinghs

This comment has been minimized.

Copy link

mohitsinghs commented Mar 8, 2019

@kaste That gist was some quick implementation based on suggestions of @getify and I did some silly mistakes, hence I failed to implement precisely what he said but got the rough idea of proposed implementation from his suggestions.

I tried putting your plumbing vehicles together in #271 😂 for that and now, We are using config lookup just for deciding executable.

Some quirks

  • self.settings.get('file_path') was throwing error
  • os.path.realpath('.') was evaluating to Applications/Sublime Text.app/Contents/MacOS

So, I relied on

  • os.path.abspath(os.path.dirname(self.view.file_name())) for getting directory for current file
  • self.get_working_dir(self.settings) for getting current working directory

For package.json I used self.manifest_path and self.get_manifest()

Explanation of changes

15efecb Searches for config and executable up to home directory.

f17360d adds the behavior to stop searching for config in a directory where eslint executable is found.

b143826 removes needs of looking global executable once local one is found with a config and adds check for setting disable_if_not_dependency that's already implemented in NodeLinter.

281fede adds one more thing from NodeLinter, respecting edge case of eslint linting itself and checks for executable binary in package.json.

@kaste

This comment has been minimized.

Copy link
Contributor

kaste commented Mar 8, 2019

Okay, cool. Before I do the code review...

The normal executable finding is done via

    for path in paths_upwards(path):
        node_modules_bin = os.path.join(path, 'node_modules', '.bin')
        executable = shutil.which(npm_name, path=node_modules_bin)

T.i. we check for each dir upwards. First question: should we just go upwards until 'HOME'?

Second: Isn't it more reasonable to go upwards like that

def paths_upwards_conatining_file(path, filename='package.json'):
    # type: (str, str) -> Iterator[str]
    for path in paths_upwards(path):
        if os.path.exists(os.path.join(path, filename)):
            yield path

and then to always read that 'package.json' evaluating if it has 'eslint' as a 'devDep' (and/or a 'bin'->'eslint' or a 'eslintConfig' key)? Looking at devDep seems like a strong indicator that 'eslint' is at least used as a tool. (This is still not sufficient because it can be used for testing only, but it narrows the set of candidates.) (This way we could probably also detect if the user still needs to 'npm install' before linting works as expected.)

Third: Should we stop looking for configs at the position where we found the local eslint (basically making the path containing 'package.json' something like the root of the project), or should we traverse up until HOME? ('eslint' doesn't know of this boundary and will just climb up.)


APPENDIX: The current behavior

It took me quite a while today to figure what the original implementation is, so for reference, this plugin currently does the following:

def paths_with_installed_package_json(start_dir, npm_name='eslint'):
    # type: (str, str) -> Iterator[str]
    for path in paths_upwards(start_dir):
        if (
            os.path.exists(os.path.join(path, 'package.json'))
            and os.path.exists(os.path.join(path, 'node_modules', '.bin'))
        ):
            yield path


def find_local(start_dir, npm_name='eslint'):
    # type: (str, str) -> Optional[str]
    manifest_path = next(paths_with_installed_package_json(start_dir), None)
    if not manifest_path:
        return None

    try:
        manifest = read_json_file(os.path.join(manifest_path, 'package.json'))
    except Exception:
        ...
    else:
        try:
            return os.path.normpath(os.path.join(manifest_path, manifest['bin'][npm_name]))
        except KeyError:
            ...

    for path in paths_upwards(manifest_path):
        executable = shutil.which(npm_name, path=os.path.join(path, 'node_modules', '.bin'))
        if executable:
            return executable
    else: 
        return None
@mohitsinghs

This comment has been minimized.

Copy link

mohitsinghs commented Mar 9, 2019

First of all, this problem exists with Microsoft/vscode-eslint/issues/549 too.

Should we just go upwards until 'HOME' ?

Should we stop looking for configs at the position where we found the local eslint (basically making the path containing 'package.json' something like the root of the project), or should we traverse up until HOME? ('eslint' doesn't know of this boundary and will just climb up.)

I tried looking into eslint code and it uses personal config ( ~/.eslintrc or others ) only if no config is found going upwards. See here. So, in my opinion we should match eslint behavior and look up to root of file system and we should provide an option to use global eslint if the only config found is personal config, for people like me and @getify. Something like use_global_for_personal_config.

Isn't it more reasonable to go upwards like that ? and then to always read that 'package.json' evaluating if it has 'eslint' as a 'devDep' (and/or a 'bin'->'eslint' or a 'eslintConfig' key)?

It is a good idea to check devDependency field or bin -> eslint and we need to update the behavior of disable_if_not_dependency to match this ( Currently it just relies on the resolution of local eslint ). The only drawback is that it does not cover all cases as you said earlier but resolution of executable will be more reliable in this way.

Should We just implement use_global_for_personal_config part and update the behavior of behavior of disable_if_not_dependency ?

@kaste

This comment has been minimized.

Copy link
Contributor

kaste commented Mar 11, 2019

Even using pen and paper I did not find a good implementation yet. Keep in mind that I need to resolve all open issues at once if I do or pull a rewrite here.

Current problems:

  • The specialized Eslint class here still must or should derive from NodeLinter even if we rewrite NodeLinter from scratch.
  • Users can actually provide '--config' manually, and I don't imagine what this does to our heuristic here.

Unfortunately, the proposed solution here doesn't really solve the problem as I would have hoped. There is also not a very good description for the issue bc it got too ranty from the beginning.

To summarize: The plugin actually works okay without a local config. For personal try-out code it usually picks the global eslint with your personal config just fine. It doesn't work as well if you have 'eslint' as a dep or devDep. (peerDeps are actually okay again because npm will not install them.)
We could restrict to looking at 'devDep' but we sure cannot differentiate any further e.g. if the 'devDep' is actually a 'testDep'.

The current solution is to set 'executable' manually, usually you would do this on a per-project basis not globally bc you're fixing an edge-case of that particular project. (I absolutely see no case why you should manually uninstall something here, or create cyclic symlinks, that's just misleading.)

The other proposed solution is to somehow match the executable and its config.

Unfortunately this doesn't solve the issue at hand which was: If I checkout a random npm project which has the said properties, eslint is not working and I am confused ...

Now if I try your PR, then I checkout @getify's eslint rule plugin because I want to hack on it, it will sure choose my global eslint and my personal configuration but of course my personal configuration doesn't match @getify's. So even though 'it' is (maybe) running, I only see red squiggles which I must happily ignore. (Because elsint is also very complicated, it is also possible that I actually get parser errors bc my personal config doesn't enable the right parser, babel etc.) The apparent solution is: Usually, if a package from a stranger doesn't come with eslint configuration, don't eslint it, disable yourself.

(Please note that this completely different from when I checkout my own code bc then I already know my own idiosyncratic project structure with its own merits.)

The problem I have here is that I can't match this behavior with what 'beginners' should expect. If you just start with 'js' it must be possible to just put eslint globally, and stray configs or no configs whereever you want until you find a manageable structure for yourself.

Something what will probably help a lot of users is, since we generally prefer local eslint's, look at 'devDep' and if there is one not yet installed, skip linting. This helps a lot I think bc then you can just checkout a random project, e.g. babel, and look at it in Sublime without eslint kicking in at all. The usual random project like babel comes with local config but right now SublimeLinter will use that local config with the global executable which is a mismatch (unless you're lucky). (FWIW only looking at 'devDep' doesn't work bc users can actually depend on something like 'gulp-eslint'.)

We can maybe also say: if there is a 'devDep' installed without a matching config, skip linting.

Examples:

  • I checkout @getify's eslint rule. If I just open subl on it I get 16 warnings plus 24 errors for the main 'lib/index.js'. After 'npm install' this doesn't change at all. The PR also doesn't change anything.

  • I checkout 'babel' (monorepo, has eslint config). I'm interested in the decorator code. If I open the main 'decorator-plugin/src/index.js' eslint crashes bc the local config requires a plugin I don't have globally. The PR doesn't change that of course. (The only way to not crash eslint is to pass '--no-eslintrc' here, but that doesn't make the experience better bc then eslint just returns a parser error on the first 'import' statement.) After 'npm install' it works just fine.

Other repos:

  • eslint-plugin-react-hooks (subpackage of a monorepo, only has peerDep, no config) (the root package has devDep plus config)
  • eslint-config-google (as @getify's plugin, devDep, no config)
  • airbnb (subpackage of a monorepo, devDep, with local config) (root package has no eslint dep)
  • gulp-eslint (direct dep, local config)
  • polymer (local config, no eslint dep, but requires gulp)
@getify

This comment has been minimized.

Copy link
Author

getify commented Mar 11, 2019

If I just open subl on it I get 16 warnings plus 24 errors for the main 'lib/index.js'. After 'npm install' this doesn't change at all. The PR also doesn't change anything.

This isn't an accurate stating of my problem, and I think it's indicative of your desire to paint my attempts here as "ranting" rather than understand them. I don't appreciate being treated as if I either don't know what I'm talking about or that I'm acting in bad faith.

I only posted the longer messages in response to your initial message that indicated you had completely misunderstood any of what my OP was talking about. I thought you would benefit from having more detail, but that obviously was the wrong move.

I'm unsubscribing from this thread because it's clearly not moving in a productive direction for the issues I'm facing.

@kaste

This comment has been minimized.

Copy link
Contributor

kaste commented Mar 11, 2019

Sure, I tried to move it but I admit I never felt I fully grasp it.

@getify

This comment has been minimized.

Copy link
Author

getify commented Mar 12, 2019

OK, I'll try to summarize one more time.

Scenario 1:

  • eslint installed globally
  • eslint plugins installed globally
  • eslint configured globally (like home dir), including loading those globally installed plugins
  • switch into project directory which has a local eslint binary, but NO local eslint config
  • Sublime-eslint running local eslint binary completely fails, because by default it's trying to use that global config, which tries to load globally installed eslint plugins, but the local binary can't see them.
  • bad fixes:
    • make a temporary local eslint config that doesn't load the plugins
    • install those eslint plugins locally (or make symlinks)
    • configure 'executable' of Sublime-eslint specifically to global eslint binary (unfortunately creates Scenario 2 below)

Scenario 2:

  • eslint installed globally
  • to avoid problem with Scenario 1, configure Sublime-eslint's 'executable' to specifically use that global eslint binary
  • switch into project directory which has a local eslint config, AND locally installed eslint plugins, AND (likely) a local eslint binary (which is moot because it won't be used because of the Sublime-eslint 'executable' config override)
  • Sublime-eslint running global eslint binary completely fails, because the global binary by default is trying to use that local config, which tries to load local plugins, but the global binary can't see those local plugins.
  • bad fixes:
    • install all those local plugins as global as well (or worse, create symlinks)
    • undo the Sublime-eslint 'executable' config, which then re-introduces Scenario 1

I don't see any way logically to resolve both of those scenarios unless you did as I suggested up-thread, which was to re-implement a "config-resolution" algorithm, to forcibly tell eslint which config to use, so that you can make sure that configs and binaries always match (either local/local, or global/global, but never mixed local/global or global/local).

@kaste

This comment has been minimized.

Copy link
Contributor

kaste commented Mar 12, 2019

Glad you're back!

  • The manual 'executable' workaround should be applied 'locally', LOL?, t.i. not in the 'SublimeLinter.sublime-settings' but in a '*.sublime-project' file. So you apply the workaround only for the hopefully few projects where you have such an untypical devDep on eslint. Such a project file minimally looks like
{
    "folders": [
        {
            "path": "."
        },
    ],

    "settings": {
        "SublimeLinter.linters.eslint.executable": "/here/we/go/eslint"
    }
}

With that, all 'typical' projects/folders use the automatic behavior we ship, and some 'untypical' projects stand on their own.

  • The '--config' setting eslint has is very weird. It doesn't force eslint to use (only) that config. It actually, as I understand it, puts that config file on top of all the other configs eslint finds locally except if it only finds a so-called personal config. T.i. to what I know, if there is actually no local config, we don't need to add the '--config' option, it will use the personal one anyway. And if there is any local config, setting '--config /home/eslintrc' just merges local and global config.

About the local/global match matrix: No question l/l and g/g are reliable matches. But you can have a batteries-included global installation and then pick what you want configs in your personal projects. This is good for beginners, personal projects, try-out code.

I think maybe only a personal config used with a local installation has maybe/probably never a valid use-case.

Now, that's exactly the case for your arrow plugin. It has unfortunately a devDep on eslint (used for testing not for tooling), and it doesn't ship an eslintrc. In my previous comment I added user stories,

I checkout @getify's eslint rule [to hack on it]. If I just open subl on it I get 16 warnings plus 24 errors for the main 'lib/index.js'. After 'npm install' this doesn't change at all. The PR also doesn't change anything.

Here the I is not you, it is a user looking at your same project. We have the said local/global mismatch, and you propose that we should choose the global eslint in that situation, and actually while playing with the PR and trying to improve it, I thought well that's a pretty arbitrary decision, the user here, she would probably be happy if we didn't lint at all.

@getify

This comment has been minimized.

Copy link
Author

getify commented Mar 12, 2019

BTW I don't actually use sublime projects (like with project configs) since the vast majority of my dev is on my own OSS projects. That contributes to why the current behavior feels so all or nothing to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.