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

PSA: ESLINT_USE_FLAT_CONFIG is only evaluated when the daemon starts/restarts #281

Open
higherorderfunctor opened this issue Nov 30, 2023 · 5 comments

Comments

@higherorderfunctor
Copy link
Contributor

higherorderfunctor commented Nov 30, 2023

Sharing to hopefully save someone else from troubleshooting if they find this by searching.

I was seeing errors like the ones below after things had been working with the new config format. The issue was I launched nvim/ale which started the daemon automatically when opening a source file without ESLINT_USE_FLAT_CONFIG. Finally solved with a ESLINT_USE_FLAT_CONFIG=true eslint_d restart.

While obvious in hindsight, it took awhile to figure out what was going on.

Additionally, it seems the config style used by the daemon is locked to whatever option was used when the daemon last started/restarted. If I have two projects open, one with the new config format and one with the old format, I will get config errors on the project that doesn't match the mode the daemon is running in.

That said, the "sticky mode" probably saved me a headache with nvim/ale. I prefer to run everything from node_modules and to not install anything globally. Everything works out of the box (so long as the daemon is the correct mode) without having to mess with ale_javascript_eslint_use_global and ale_javascript_eslint_executable in my nvim config.

Only actionable item I could potentially see is to auto-detect the config format on invocation. That said, if it is not worth the effort, I can live with the limitations now that I understand them.

$ ESLINT_USE_FLAT_CONFIG=true eslint_d --cache .

Error: No ESLint configuration found in /path/to/project
$ ESLINT_USE_FLAT_CONFIG=true eslint_d --cache -c eslint.config.js .

Error [ERR_REQUIRE_ESM]: Cannot read config file: /path/to/eslint.config.js
Error: require() of ES Module /path/to/eslint.config.js from /path/to/node_modules/@eslint/eslintrc/dist/eslintrc.cjs not supported.
Instead change the require of eslint.config.js in /path/to/node_modules/@eslint/eslintrc/dist/eslintrc.cjs to a dynamic import() which is available in all CommonJS modules.
@mantoni
Copy link
Owner

mantoni commented Nov 30, 2023

Thank you for reporting and also providing a solution! 😍

A potential solution to this could be the idea to launch a separate process per project as suggested here, which might be the way forward anyway.

In the meantime, can I ask you to send a PR for the README.md to point to this issue in the section where ESLINT_USE_FLAT_CONFIG is mentioned? 🙏

@higherorderfunctor
Copy link
Contributor Author

Opened #282 with the README.md addition.

I would be in favor of separate processes. I do wonder if this may cause excessive instances in some situations. I don't think it would impact my typical workflow, but here are a few things I checked:

A monorepo sharing the same hoisted install. If packages require different config formats, it would be useful. If they do not, it may cause different instances when not needed. I typically only add linters as a dependency and lint from the root project. I'm just not sure what percentage of users do the same.

pnpm seems to always set the cwd to the location of the package.json when running something from scripts, which is good, but I am also not sure if all package managers behave this way.

ale also looks okay for at least eslint. :ALEInfo shows the cwd used with eslint as always the monorepo root even when I cd into a sub project before launching nvim (though it cannot find the config as it looks in the sub-project for it). I don't ever do this... so no big deal, but again, my workflow may not represent everyone else's.

@mantoni
Copy link
Owner

mantoni commented Dec 1, 2023

I would be in favor of separate processes. I do wonder if this may cause excessive instances in some situations.

Separate instances could also make the proposed solution for #236 easier to implement, where eslint_d would allow to dispose an instance for a given working directory. Editors could be instructed to call eslint_d evict . on exit which would simply kill the process associated with the cwd.

@higherorderfunctor
Copy link
Contributor Author

Separate instances could also make the proposed solution for #236 easier to implement, where eslint_d would allow to dispose an instance for a given working directory. Editors could be instructed to call eslint_d evict . on exit which would simply kill the process associated with the cwd.

Okay, I could see that working! It would be a great stepping stone at a minimum to tackle some of the issues.

Brain dumping a couple thoughts, but I wouldn't call them requirements for the core idea.

  • An optional TTL for each daemon may help with the rogue process potential. That is, after a certain amount of inactivity, the daemon self evicts.
  • I'm not up to date on how LSPs communicate with editors, but I wonder if there is potential there. I know the ts-server instance used by https://github.com/neoclide/coc.nvim is tied to the editor process. Killing (n)vim gracefully or forcibly will cause the ts-server instance to exit. On the other hand, supporting users who use both their editor and manual invocations via CLI could be tricky when deciding to use the same instance and if the instance should be preserved when the editor exits. Like-wise, how trivial or non-trivial an LSP implementation is would also be a major factor. Regardless, figured I would share, though it is probably overkill.

@mantoni
Copy link
Owner

mantoni commented Dec 4, 2023

Thanks for sharing your thoughts, @higherorderfunctor.

  • The TTL idea could be opt-in, either with an option to eslint_d start or another environment variable.
  • I don't see how building an LSP would fit together with eslint. The protocol is useful for compilers, but I don't see how it would help with linters.

As far as I understand coc.nvim seems to launch the process from within a vim plugin, which ties the process to vim. If we'd like to achieve the same, we'd have to create a vim plugin that launches a new eslint_d instance on demand.

This leads to an interesting design decision: If we want separate processes per project, is the eslint_d client creating these processes? Or would we have a "service" process that controls the "worker" instances?

If eslint_d processes would operate completely independent from each other, that would probably be harder to manage (socket connection, port, security ...), but it would make it simpler to tie the process to an editor. On the other hand, having a "service" process would add complexity to dispatching invocations from the client through the service to the corresponding "worker".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants