Skip to content

Default projectService to false in eslint adder #631

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ieedan
Copy link
Contributor

@ieedan ieedan commented Jul 12, 2025

In every single project I maintain I make sure this is off because if it's on I have seen linting take as long as 3 minutes, quickly increasing with the size of the project I feel like there is a pretty good case for having this off by default unless it catches something extra that is really important.

There has been an open issue for this for a while in eslint-plugin-svelte but it has kinda stagnated: sveltejs/eslint-plugin-svelte#1084

Copy link

changeset-bot bot commented Jul 12, 2025

🦋 Changeset detected

Latest commit: 5586cd2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
sv Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

pkg-pr-new bot commented Jul 12, 2025

Open in StackBlitz

npx https://pkg.pr.new/sveltejs/cli/sv@631
npx https://pkg.pr.new/sveltejs/cli/svelte-migrate@631

commit: 5586cd2

Copy link
Member

@benmccann benmccann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a good idea. projectService: true is the recommendation of the typescript-eslint team. And it's generally faster - in fact a benchmark from SvelteKit was quoted in their blog post showing that it was. If you have a case where it's slower, the typescript-eslint team is very interested in bug reports on that topic.

I'm not sure this is related to sveltejs/eslint-plugin-svelte#1084 since it says:

This occurs with both projectService: true and project: './tsconfig.json'

We should fix sveltejs/eslint-plugin-svelte#1084 though. There was some good investigation there last month as to the potential cause and we should dig into it further

@ieedan
Copy link
Contributor Author

ieedan commented Jul 13, 2025

I don't think this is a good idea. projectService: true is the recommendation of the typescript-eslint team. And it's generally faster - in fact a benchmark from SvelteKit was quoted in their blog post showing that it was. If you have a case where it's slower, the typescript-eslint team is very interested in bug reports on that topic.

I'm not sure this is related to sveltejs/eslint-plugin-svelte#1084 since it says:

This occurs with both projectService: true and project: './tsconfig.json'

We should fix sveltejs/eslint-plugin-svelte#1084 though. There was some good investigation there last month as to the potential cause and we should dig into it further

Every project I have tried this on it's slower so maybe it's worth opening an issue with typescript-eslint.

There's actually a comment here https://github.com/huntabyte/shadcn-svelte/blob/main/eslint.config.js#L30 mentioning it's poor performance

@AdrianGonz97
Copy link
Member

AdrianGonz97 commented Jul 16, 2025

It seems like projectService: true is only required when we're using typed linting.

Since we're only using ts.configs.recommended here (which doesn't include any type-aware rules), instead of ts.configs.recommendedTypeChecked, projectService: true doesn't need to be specified.

with that in mind, i think we have 2 options:

  1. use ts.configs.recommendedTypeChecked and keep projectService: true enabled
  2. or keep ts.configs.recommended and remove projectService: true

@benmccann
Copy link
Member

benmccann commented Jul 16, 2025

I know that in Svelte and SvelteKit we've turned on a couple of extra rules that are type-checked:

'@typescript-eslint/await-thenable': 'error',
'@typescript-eslint/require-await': 'error'

Some of the stuff that came in recommendedTypeChecked caused too many false positives in browser-based code, which is why we didn't use that. Though we also turned off tons of stuff from recommended as well. The base eslint recommended configs are pretty good, but the typescript-eslint ones leave a lot to be desired. I don't know that I necessarily want to get too opinionated about selecting specific rules though as it opens up an opinionated area of debate. Maybe just a comment the existing code with something like:

// you may also want to consider adding some of typescript-eslint's type-checked rules

@manuel3108 manuel3108 added enhancement New feature or request pkg:add sv add labels Jul 18, 2025
@manuel3108
Copy link
Member

@baseballyama What would be your preffered way here?

@baseballyama
Copy link
Member

I’m against setting projectService to false by default. Many type-aware rules will be lost that way. So, I think the appropriate starting point is projectService: true.

eslint-plugin-svelte converts Svelte files into virtual code and then uses the TypeScript ESLint parser to obtain type information. In this process, type inference does a lot of work, so it takes about 2 seconds per file. This is the reason why eslint-plugin-svelte is slow.

On the other hand, we can speed up ESLint significantly just by running it in parallel, without setting projectService: false.
In fact, my project currently has 1,134 components and 333 rules, but the process completes in about 4 minutes and 30 seconds, because I run ESLint in parallel across all CPU threads. If we increase the number of cores, it will be even faster.

@AdrianGonz97
Copy link
Member

+1 for @benmccann's suggestion

having it commented out with a short description of what it's for does seem like a nice middle ground since, in its current state, all it does is slow down the linting by a considerable amount, even when no type-aware rules are enabled in the default config that we currently apply

On the other hand, we can speed up ESLint significantly just by running it in parallel, without setting projectService: false.

do you have an example project with this setup? seems like a great solution for large projects, though it might be too advanced for our simple scaffolding

@baseballyama
Copy link
Member

In other words, I don’t think we need to immediately set projectService: false just because the linter is slow. There are still options like using the ESLint cache or running it in parallel.
So, at the scaffolding stage, we should set projectService: true, and if linting is slow, users can choose measures such as enabling the ESLint cache, parallel execution, or setting projectService: false.

do you have an example project with this setup?

This issue is also being discussed in the eslint-plugin-svelte repository, so I’d like to share a sample code via Gist. However, I’m short on time until Sunday, so please wait a little longer.

This is a separate topic, but I’m hoping that oxlint will be able to support Svelte as well.

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

Successfully merging this pull request may close these issues.

5 participants