Bug: Linter doesn't pick up node_modules changes

[Standard8]

Issue Type

Bug

Issue Description

• Have a repository that is set up for ESLint.
• Have Atom loaded on that repository.
• On disk, install a new ESLint module.
• Add code to the ESLint configuration to use a rule from that module.

Actual Results:

Restarting Atom fixes the issue.

Expected results:

Either of:

• The linter detects the change to node_modules and reloads the node process.
• The issue is somehow detected and a Restart button is provided.

Bug Checklist

• Restart Atom
• Verify the eslint CLI gives the proper result, while linter-eslint-node does not
• Paste the output of the Linter Eslint Node: Debug command from the Command Palette below

Linter Eslint Node: Debug output here

Atom version: 1.60.0
linter-eslint-node version: 1.0.3
Worker using Node at path: /usr/local/bin/node
Worker Node version: v17.2.0
Worker PID: 2958
ESLint version: 8.9.0
ESLint location: /Users/mark/dev/thunderbird-conversations/node_modules/eslint/
Linting in this project performed by: linter-eslint-node
Hours since last Atom restart: 0.1
Platform: darwin
Current file's scopes: [
  "source.js"
]
linter-eslint-node configuration: {
  "disabling": {
    "disableWhenNoEslintConfig": true,
    "rulesToSilenceWhileTyping": []
  },
  "scopes": [
    "source.js",
    "source.jsx",
    "source.js.jsx",
    "source.flow",
    "source.babel",
    "source.js-semantic",
    "source.ts"
  ],
  "nodeBin": "node",
  "warnAboutOldEslint": true,
  "autofix": {
    "fixOnSave": false,
    "rulesToDisableWhileFixing": [],
    "ignoreFixableRulesWhileTyping": false
  },
  "advanced": {
    "disableEslintIgnore": false,
    "showRuleIdInMessage": true,
    "useCache": true
  }
}

[savetheclocktower]

This is supposed to work because we clear the instance cache whenever we detect a change to an .eslintrc. We don't restart the worker process in that case, but I wouldn't have thought that we'd need to. I'll see if I can reproduce this. Thanks!

[savetheclocktower]

This one is working for me. I'm stumped.

My attempt to reproduce it:

1. Start with a barebones project after npm init --yes; add a couple of source files to it and a basic .eslintrc
2. atom --dev .
3. Verify linting works
4. npm install --save-dev eslint-plugin-import
5. Without reloading the Atom project, change the .eslintrc to reference "plugins": ["import"] and add "import/newline-after-import": "error" to the rules section
6. Save the .eslintrc; no error happens for me at this point
7. Go to a source file and remove a newline between an import and something else
8. Observe the new linting error (if “Fix on Save” is disabled) or the code being fixed (if it isn't). I tried both scenarios and they both worked for me.

I'm using linter-eslint-node version 1.0.5 on macOS. Only thing I can think of is this: is your config named .eslintrc.js or .eslintrc.yml, perhaps? The code is written such that a change to any of those ought to clear the instance cache, but maybe there's an oversight.

This also might be an error that doesn't happen in the general case, but rather because of some confluence of conditions that my test case isn't meeting. Are there any further details about the error in the console?

[Standard8]

Thinking about this, I wonder if my STR are slightly wrong. I'm thinking that what I've actually been doing is to apply a patch or switch to a branch which updates .eslintrc.js and package.json and only after that npm ci is run. That would defeat the restart based on .eslintrc.js changing.

[savetheclocktower]

We're using Project#onDidChangeFiles here, so it ought to detect any filesystem changes underneath the project directory. It's not dependent on whether the change to .eslintrc was made within the editor.

I specifically had to put in a guard to prevent it from reacting to .eslintrcs inside node_modules, because an npm install was triggering cache clearance many, many, many times in quick succession. I have no reason to think a git-initiated filesystem change would not also be picked up on, but it's another thing I can try.

[Standard8]

Is there any delay / de-bouncing on when the reload is triggered? mozilla-central is quite a large repo, and I just tried changing branches (across a .eslintrc.js change) and got three linter-eslint-node Error messages on the UI.

In the developer tools console, there were two of these messages:

Uncaught (in promise) Error [ERR_STREAM_DESTROYED] [ERR_STREAM_DESTROYED]: Cannot call write after a stream was destroyed
    at doWrite (_stream_writable.js:431)
    at writeOrBuffer (_stream_writable.js:419)
    at Socket.Writable.write (_stream_writable.js:309)
    at /Users/mark/.atom/packages/linter-eslint-node/lib/job-manager.js:197
    at new Promise (<anonymous>)
    at JobManager.<anonymous> (/Users/mark/.atom/packages/linter-eslint-node/lib/job-manager.js:194)
    at Generator.next (<anonymous>)
    at step (/Users/mark/.atom/packages/linter-eslint-node/lib/job-manager.js:13)
    at /Users/mark/.atom/packages/linter-eslint-node/lib/job-manager.js:13
    at new Promise (<anonymous>)
    at JobManager.<anonymous> (/Users/mark/.atom/packages/linter-eslint-node/lib/job-manager.js:13)
    at Object.clearESLintCache (/Users/mark/.atom/packages/linter-eslint-node/lib/main.js:246)
    at /Users/mark/.atom/packages/linter-eslint-node/lib/main.js:180
    at Function.simpleDispatch (/Applications/Atom.app/Contents/Resources/app/static/<embedded>:11)
    at Emitter.emit (/Applications/Atom.app/Contents/Resources/app/static/<embedded>:11)
    at s (/Applications/Atom.app/Contents/Resources/app/static/<embedded>:11)
    at PathWatcher.onNativeEvents (/Applications/Atom.app/Contents/Resources/app/static/<embedded>:14)
    at /Applications/Atom.app/Contents/Resources/app/static/<embedded>:14
    at Function.simpleDispatch (/Applications/Atom.app/Contents/Resources/app/static/<embedded>:11)
    at Emitter.emit (/Applications/Atom.app/Contents/Resources/app/static/<embedded>:11)
    at NSFWNativeWatcher.onEvents (/Applications/Atom.app/Contents/Resources/app/static/<embedded>:14)
    at watcher.c.debounceMS (/Applications/Atom.app/Contents/Resources/app/static/<embedded>:14)

I'm not sure it is strictly related to this issue, but seems relevant to the discussion.

[savetheclocktower]

That's interesting to me because it's trying to write to stdout after a worker has been killed. I thought that I'd covered all those cases. I'm also not sure what would trigger the worker to get killed in this scenario; the cache can get cleared without needing to start a new worker. A guard before the write on this line would probably fix the proximate error, but it wouldn't solve the mystery.

There is no delay or de-bounce when cache is being cleared, and I agree that there should be. (I'd wager that, in your case, this loop is clearing the cache more than once in quick succession in the case of a branch change.) But I can't say if that would fix this problem until I understand what's killing that worker. What I should definitely do is put in a hidden setting that would enable the verbose logging that otherwise is reserved for dev mode.

[nelson6e65]

I'm using multiple .eslintrc.json and npm workspaces. This often happens in my functions workspace.

I disabled the cache, but I'm getting the same error:
Error [ERR_STREAM_DESTROYED]: Cannot call write after a stream was destroyed