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

eslint-lsp over eslint_d in the typescript pack #593

Closed
willparsons opened this issue Oct 1, 2023 · 6 comments · Fixed by #600
Closed

eslint-lsp over eslint_d in the typescript pack #593

willparsons opened this issue Oct 1, 2023 · 6 comments · Fixed by #600
Labels
enhancement New feature or request

Comments

@willparsons
Copy link

Is your feature related to a problem?

When using eslint_d via null-ls it means you cannot configure eslint like the lsp so you are limited on how you use it.

Describe the new feature

  • Remove eslint_d
  • Add eslint-lsp (alongside tsserver)

Additional context

No response

@willparsons willparsons added the enhancement New feature or request label Oct 1, 2023
@Uzaaft
Copy link
Member

Uzaaft commented Oct 2, 2023

I don't think the reason you're stating here is a valid reason to go over to eslint-lsp. eslint_d can be configured just the same way eslint is, which many of the typescript pack users is used to and know about. Another layer of configuration in and of itself isn't an improvement.

We're(meaning mostly I) am exploring if estlint-lsp has any advantage(performance, fewer bugs, etc), so we'll have to see what fruits that exploration give and if we'll include it as the default method of using eslint in the pack in the future.

@willparsons
Copy link
Author

eslint_d can be configured just the same way eslint is

For me, I find lsp configuration simpler than null-ls, to be fair its mainly because I just don't really use null-ls. With that being said, here is the config for the eslint-lsp, how would you change these settings via null-ls? Aren't they just fundamentally different?

Another layer of configuration in and of itself isn't an improvement.

Is it another layer? Isn't it just moving the config from null-ls to lsp?

We're(meaning mostly I) am exploring if estlint-lsp has any advantage(performance, fewer bugs, etc), so we'll have to see what fruits that exploration give and if we'll include it as the default method of using eslint in the pack in the future.

Sounds good. I mainly made this issue to find out what tool is 'better' since it's not obvious at first glance.

@Uzaaft
Copy link
Member

Uzaaft commented Oct 5, 2023

The eslint-lsp branch has this if you want to test it out. We're testing it internally as well. :)

@lougreenwood
Copy link
Contributor

Dropping this question here to avoid spamming with a new issue (i'll create one as a followup if necessary), but does eslint-lsp support auto-fix on save?

I recently made my eslint & eslint-typescript configs more strict and I'm now seeing rules which are fixable not being fixed on save.

Is this expected using master and the latest version of the typescript pack?

@Uzaaft
Copy link
Member

Uzaaft commented Nov 28, 2023

Dropping this question here to avoid spamming with a new issue (i'll create one as a followup if necessary), but does eslint-lsp support auto-fix on save?

I recently made my eslint & eslint-typescript configs more strict and I'm now seeing rules which are fixable not being fixed on save.

Is this expected using master and the latest version of the typescript pack?

Fix on save was never added back in when we migrated eslint-lsp. See #608 for the currect tracking issue.
This will most likely be resolved in v4.

@willparsons
Copy link
Author

You can setup an autocommand to run :EslintFixAll

vim.api.nvim_create_autocmd("BufWritePost", {
    desc = "Fix all eslint errors",
    pattern = { "*.tsx", "*.ts", "*.jsx", "*.js" },
    group = "...",
    callback = function()
      if vim.fn.exists ":EslintFixAll" > 0 then vim.cmd "EslintFixAll" end
    end,
  })

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

Successfully merging a pull request may close this issue.

3 participants