Frequent slowdowns

[IPWright83]

I'm using eslint_d through a Sublime extension to auto format https://github.com/TheSavior/ESLint-Formatter and I'm seeing frequently that Sublime reports that it's become unresponsive, only when using said extension.

I've tried running eslint_d via the command line and noticed that while it's often quick, sometimes it runs as though it's just warming up for the first time, and therefore takes 30s+ (unfortunately in my project :( ). This is when I've been saving the same file multiple times so it should have already been cached.

Is there anyway I can debug/find out more information as to why I'm hitting these frequent slow-downs on files that should be cached? I've seen there's eslint_d status, but it doesn't tell me a huge amount, and running before every file save is going to be awkward :/

[IPWright83]

Potentially related, I've noticed that as I pull out tabs from Sublime into new windows it seems to create a new eslint_d instance for each, and the extension I'm using never closes them down.

❯ eslint_d status Running. 2 instances cached.

I managed to get a reproduction when creating a 2nd instance. I'm not sure if this is eslint_d at fault or the extension I'm using though.

[IPWright83]

Thought I should share my config:

"node_path": {
        "linux": "/home/ian/.nvm/versions/node/v16.14.0/bin/node"
    },
    "local_eslint_path": {
        "linux": "/home/ian/.nvm/versions/node/v16.14.0/bin/eslint_d"
    }

I'm wondering if installing eslint_d locally to my project may help with this as at the moment I'm using a shared one.

[mantoni]

Hmm, eslint_d creates a new instance if the current working directory changes. Maybe the cwd is not the project root when you pull out a file into a new window?

Unfortunately it's a bit tricky to get information from the daemon. I thought about a --logfile option for debug logging, but didn't get to it yet.

[IPWright83]

Yeah, I suspected that might be the case when I saw https://github.com/TheSavior/ESLint-Formatter/blob/master/ESLint-Formatter.py#L73 but I tried logging the commands used and I couldn't see the cwd being passed in anywhere.

That being said, I noticed that the fix results were also different, the window'd version was fixing to I'm guessing the default set of rules, where-as the project was fixing to the .eslintrc config which was subtly different.

The plugin I'm using doesn't seem to get much attention at the moment, so I'm currently trying to work out ways I can debug & potentially resolve this. I'm guessing if I were to use --resolve-plugins-relative-to and hard-code to my cwd then I'd only ever have 1 instance of eslint_d? Problem is I'd only then be able to run it for a single project :/

[IPWright83]

I've confirmed that this comes down to the cwd. When this is working correctly the cwd is the top level directory of my project, where-as when it's broken in a separate window it's treating the cwd as the directory in which the file lives in.

I've been mulling over how I could fix this, and I reckon walking back up the directory structure until a package.json is found, then treating that as the cwd would fix my issue. Would you be open to considering that? Possibly with a new CLI switch to select that behaviour to ensure this is non-breaking for existing users?

[IPWright83]

I reckon something like this would work - just not 100% sure where the best place to put it is? The idea being that if you've enabled the flag, it'll call this function - which if it returns a non-null value should then set the cwd to the given value.

Could tweak it to instead look for eslint config, but there's a lot more permutations to cope with there.

const fs = require('fs');
const path = require('path');

function findPackageJsonDir(currentDir) {
    const containsPackageJson =
        fs.readdirSync(currentDir).filter((f) => f.includes('package.json')).length > 0;

    if (containsPackageJson) {
        return currentDir;
    }

    // Check for hitting the root without finding a package.json
    const parent = path.dirname(currentDir);
    if (parent === currentDir) {
        return null;
    }

    return findPackageJsonDir(path.dirname(currentDir));
}

[mantoni]

I'm open to your suggestions. I wonder how eslint itself handles this. Ideally eslint_d behaves the same, or even uses eslint APIs to find the cwd. If that's possible, I wouldn't even put it behind a flag. If eslint_d behavior behaves potentially different from eslint, I think it should be behind a flag.

[IPWright83]

I've checked eslint and unfortunately it suffers from the same behaviour (not picking up rules from the cwd). There's a lot more code in there to try and follow but I've read some issues about people passing the cwd in as an argument.

I think the thing with eslint_d is one of it's strong points is replacing eslint when running in an editor/IDE to get rapid feedback, but configuring the cwd per project is often not possible (certainly not in my editor at least). So I'd probably encourage the flag approach - but you're call.

If you can give me some pointers on where it should live then I can try putting together a PR sometime next week maybe?

[mantoni]

Another idea: How about checking if the current cwd is a sub-directory of one of the cached instances?

[IPWright83]

That'd work well - probably an easier change to make too?

[mantoni]

Easy to implement, and also a synchronous check without additional IO.

I'm a little worried though. This might cause issues if an instance is cached in my home directory and all of the project folders would then use that instance 🤔

[mantoni]

In any case, the place to look at in the code is lib/caches.js in the getCache function.

You could implement some sort of "project root path resolution" there, behind a flag.

[IPWright83]

Easy to implement, and also a synchronous check without additional IO.

I'm a little worried though. This might cause issues if an instance is cached in my home directory and all of the project folders would then use that instance thinking

That's true - but I guess an easy workaround? eslint_d stop then re run in the correct project folder? Then you've set up a new set of caching.

[mantoni]

I thought some more about it and it could also cause unexpected behavior in mono-repo project setups.

I'm okay with supporting a new command line flag for project root resolution. This will fix it for you and doesn't change the caching behavior for anyone else.

[IPWright83]

Cool, thanks @mantoni. Better to get your steer before I try building it.

[IPWright83]

So I've discovered you can set the working directory externally when launching eslint_d as used by another plugin. I feel like this might be a cleaner approach, and not require any changes to eslint_d so I'm going to try that approach first.

[mantoni]

Maybe this is also interesting for you: https://github.com/mantoni/eslint_d.js/issues/203

[IPWright83]

@mantoni that looks like a similar problem to the one I hit. The problem is the path to the project/node_modules/eslint_d is fixed for each project, but this is a global setting for the plug-in. I tried to fix it in a slightly more generic way using the same approach we were going to do with eslint_d but pushed it into the plugin instead.

https://github.com/TheSavior/ESLint-Formatter/pull/90

[mantoni]

I guess this can be closed as resolved now, right? Thank you for all the work ❤️

[IPWright83]

Yeah, thanks @mantoni . I still get issues for new files unfortunately that haven't been cached yet which there's not much can be done. But I think the other problems are now gone.

[mantoni]

Thanks for the feedback. If you have ideas how to further improve the situation, feel free to open a new issue.