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

feat: add svelte/component-tags-order rule #498

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

Conversation

baseballyama
Copy link
Member

close: #493

@changeset-bot
Copy link

changeset-bot bot commented Jun 4, 2023

🦋 Changeset detected

Latest commit: 5ac917d

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 Minor

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

.npmrc
tests/fixtures/rules/component-tags-order
Copy link
Member Author

Choose a reason for hiding this comment

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

How can I disable prettier for specific file?

Copy link
Member

Choose a reason for hiding this comment

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

I often use <!-- prettier-ignore -->, but it is OK to use a .prettierignore file.

@baseballyama baseballyama marked this pull request as ready for review June 4, 2023 08:33
"error",
{
"order": [
"SvelteScriptElement([context=module])",
Copy link
Member

Choose a reason for hiding this comment

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

Hmm. I don't think it's a good idea to use a format specifically for this rule.
For example, using the CSS selector format most users already know the format. (However, users may be a little surprised because the colon must be escaped. e.g. svelte\:options)
Is the format currently used by the rule inspired by something?

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 copied most parts from https://eslint.vuejs.org/rules/component-tags-order.html
And while implementing the rule, I felt that we need to support and condition, but I couldn't find the way to use and condition in the ESLint selector.
https://eslint.org/docs/latest/extend/selectors

And I think that the ESLint selector is complicated for most users because they need to understand Svelte ESLint AST.

That's why I go to use a special format.
(But yeah, I didn't support escaping things...)

Copy link
Member

Choose a reason for hiding this comment

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

And while implementing the rule, I felt that we need to support and condition, but I couldn't find the way to use and condition in the ESLint selector.

Are you saying we can't use and in CSS selectors?
I think that the and condition in the CSS selector just connects without using spaces. e.g. script[context=module][lang=ts]
And in modern CSS selectors the or condition uses :is(). e.g. :is(script, style)

https://eslint.org/docs/latest/extend/selectors

I agree that esquery is difficult for users to understand.

Copy link
Member

Choose a reason for hiding this comment

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

By the way, eslint-plugin-vue implements a function that converts CSS selectors to matchers in order to use CSS selectors in defining vue/component-tags-order rule option.

https://github.com/vuejs/eslint-plugin-vue/blob/81ce0cecb87eae60830b9b973790d4f2386062c1/lib/utils/selector.js#L22

@marekdedic
Copy link
Contributor

Hi,
to add context, please see #493 (comment)

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.

Rule Proposal: svelte/component-tags-order
3 participants