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

breaking: Redesigned ESLint config #957

Closed
wants to merge 24 commits into from
Closed

Conversation

baseballyama
Copy link
Member

@baseballyama baseballyama commented Dec 14, 2024

Svelte 5 and Svelte 4 have different features. Additionally, Svelte 5 introduces Runes mode and legacy mode, with some features marked as deprecated.
For a new project, rules related to deprecated features and legacy mode are likely unnecessary. While the Svelte version can be automatically determined from package.json, However ESLint cannot automatically detect whether deprecated features are being used without walking the AST.
Additionally, I personally dislike longer ESLint execution times caused by unnecessary rules, so I prefer to turn off unneeded rules at the configuration level. Configuring ESLint is a rare task, so even if some adjustments are required in the next major version, I believe it’s better to prioritize faster linting in daily workflows.

To address this, this PR provides the following three configurations:

  • A recommended ruleset for all features available in Svelte 5.
  • A recommended ruleset for Svelte 5 without legacy/deprecated features.
  • A recommended ruleset for Svelte 3/4.

Alternative of #943

Copy link

changeset-bot bot commented Dec 14, 2024

🦋 Changeset detected

Latest commit: 585c1f9

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

This PR includes changesets to release 1 package
Name Type
eslint-plugin-svelte Major

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

@@ -4,6 +4,7 @@
"editor.defaultFormatter": "esbenp.prettier-vscode",
"editor.codeActionsOnSave": {
"source.fixAll": "explicit",
"source.fixAll.eslint": "explicit",
Copy link
Member Author

Choose a reason for hiding this comment

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

ESLint fix didn’t work on save for some reason, but it started working after adding this.

@@ -1,5 +1,5 @@
import path from 'path';
import { name, version } from '../package.json';
import packageJson from '../package.json' assert { type: 'json' };
Copy link
Member Author

Choose a reason for hiding this comment

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

To solve TypeScript error.

@@ -9,7 +9,6 @@ void main();
async function main() {
const { pluginsToRulesDTS } = await import('eslint-typegen/core');

// @ts-expect-error - types are a bit strict here
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't get TypeScript error here.

@baseballyama baseballyama force-pushed the breaking/update-config branch 2 times, most recently from 528bb28 to 01f686a Compare December 14, 2024 06:53
@baseballyama baseballyama force-pushed the breaking/update-config branch from 006bd91 to 4395124 Compare December 14, 2024 07:04
@baseballyama baseballyama marked this pull request as ready for review December 14, 2024 13:04
@baseballyama baseballyama mentioned this pull request Dec 14, 2024
25 tasks
Copy link

pkg-pr-new bot commented Dec 30, 2024

Open in Stackblitz

npm i https://pkg.pr.new/eslint-plugin-svelte@957

commit: 585c1f9

@baseballyama
Copy link
Member Author

I think #980 is better.

@marekdedic
Copy link
Contributor

I think #980 is better.

These are mutually exclusive, do I understand that correctly?

@baseballyama
Copy link
Member Author

Yes, if we merge #980, I will close this PR.

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

Successfully merging this pull request may close these issues.

2 participants