chore!: update recommended config

[baseballyama]

close: #755
close: #785
close: #894

[pkg-pr-new]

Open in Stackblitz

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

commit: 7d1bdf4

[changeset-bot]

🦋 Changeset detected

Latest commit: 7d1bdf4

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

[baseballyama]

@ota-meshi
The current configuration seems quite aggressive. Could you share your thoughts on which rules should be included as recommended?

[ota-meshi]

I like this new config 👍

I think it might be a good idea to remove valid-compile from the recommended config. What do you think?
This rule was added for compatibility with eslint-plugin-svelte3.

[marekdedic]

Hi, thanks, I am thinking about whether these shouldn't be added as well (in decreasing order of certainty):

• svelte/no-navigation-without-base - This one warns about what is incorrect code (though it may be fixed in SvelteKit at some point in the future)
• svelte/button-has-type - This one is about what is considered best practice
• svelte/no-unused-class-name - This one is more about unused code...

[baseballyama]

svelte/no-navigation-without-base

I don’t think this needs to be added. (In fact, it’s better not to add it.) This is because it would result in meaningless linting errors for users who don’t use basepath. Only users who use basepath should opt in to this rule.

svelte/button-has-type

This is not included in the recommended rules for Vue either. IIRC it was added to the Vue's recommended config once before but was removed due to user pushback.
vuejs/eslint-plugin-vue#1432

svelte/no-unused-class-name

This was initially added, but in the codebases I tested, false positives occurred. Therefore, I judged that it was not mature enough to be included in the recommended rules.

[marekdedic]

Thanks, in reverse order now :D

svelte/no-unused-class-name

This was initially added, but in the codebases I tested, false positives occurred. Therefore, I judged that it was not mature enough to be included in the recommended rules.

Hmm, if it has too many FPs, it shouldn't be added , I agree. Could you open an issue for the FPs? I'd like to fix them (regardless of the recommended config)

svelte/button-has-type

This is not included in the recommended rules for Vue either. IIRC it was added to the Vue's recommended config once before but was removed due to user pushback.

I can't find anything about the removal, maybe @ota-meshi can explain what the reason was?

svelte/no-navigation-without-base

I don’t think this needs to be added. (In fact, it’s better not to add it.) This is because it would result in meaningless linting errors for users who don’t use basepath. Only users who use basepath should opt in to this rule.

I am torn on this one - on the one hand, it would be more warnings for users, on the other, I consider not adding the base path to be incorrect - if the code is e.g. reused in the future, this is a serious foot-gun (So much so that the SvelteKit team considers fixing it on their end).

[baseballyama]

I am torn on this one - on the one hand, it would be more warnings for users, on the other, I consider not adding the base path to be incorrect - if the code is e.g. reused in the future, this is a serious foot-gun (So much so that the SvelteKit team considers fixing it on their end).

IMO:
Whether to reuse the code with a different basepath is up to each project. If there’s no chance of reuse, we don’t need to consider the basepath at all. If it turns out later that a basepath is needed, we can simply opt into this rule and make adjustments then.

Code is read more often than it is written, so reducing unnecessary noise is important. For projects where the basepath doesn’t need to be considered, the code added by this rule would just become noise.

[ota-meshi]

The config provided by eslint-plugin-vue follows the Vue style guide, so if something is not mentioned in the style guide, it will not be added to the config. Therefore, the vue/button-has-type rule is not included in the config.

However, neither react/button-has-type nor @angular-eslint/template/button-has-type seem to be included in the config provided by those plugins.
Therefore, it seems reasonable to not include svelte/button-has-type in the config either.

[baseballyama]

I believe we’ve agreed on the recommended config. Finally, I’ll document the changes in detail in the changeset and wrap it up.

[ota-meshi]

Thank you very much!