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

Add rule-selector-property-disallowed-list #5679

Conversation

doing-art
Copy link
Contributor

@doing-art doing-art commented Oct 31, 2021

Which issue, if any, is this issue related to?

The new rule rule-selector-property-disallowed-list that is mentioned in #5433 has been added.

Is there anything in the PR that needs further explanation?

@jeddy3 I see that the branch v14 has been merged and my previous PR #5489, targeted to that branch, has been closed. So, I am creating this one to the main branch.

I made the change that you asked me to do in comments to the previous PR. Namely, added a new test which checks multiple errors in one CSS block.


Closes #5433

Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

@doing-art Thanks for creating a new pull request

my previous PR #5489, targeted to that branch, has been closed

Yep, it was automatically closed by GitHub when v14 was merged.

I've requested one last changes, otherwise LGTM.

@jeddy3 jeddy3 changed the title Feature/add rule selector property disallowed list Add rule-selector-property-disallowed-list Oct 31, 2021
@ybiquitous
Copy link
Member

@doing-art Thanks for recreating the PR!

The current HEAD revision seems a bit old because the project version specifies 13.13.1 which is not the latest version:

https://github.com/doing-art/stylelint/blob/0fcb254923ee48e14df7efc51fe997c599d1cc74/package.json#L3

The latest version is 14.0.1:

"version": "14.0.1",

Just in case, could you rebase the main branch, please?

doing-art and others added 17 commits November 1, 2021 11:22
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
…ex.js

Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
@doing-art doing-art force-pushed the feature/add-rule-selector-property-disallowed-list branch from 993570d to 584c8c7 Compare November 1, 2021 09:22
@doing-art
Copy link
Contributor Author

@doing-art Thanks for recreating the PR!

The current HEAD revision seems a bit old because the project version specifies 13.13.1 which is not the latest version:

https://github.com/doing-art/stylelint/blob/0fcb254923ee48e14df7efc51fe997c599d1cc74/package.json#L3

The latest version is 14.0.1:

"version": "14.0.1",

Just in case, could you rebase the main branch, please?

Thank you for your comment. I rebased main.

Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

@doing-art Thank you. LGTM 👍🏼

Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

Thanks!

@jeddy3 jeddy3 merged commit 1bc40a1 into stylelint:main Nov 2, 2021
@jeddy3
Copy link
Member

jeddy3 commented Nov 2, 2021

Changelog entry added:

  • Added: rule-selector-property-disallowed-list rule (#5679).

@Mouvedia
Copy link
Contributor

@jeddy3 can we reuse rules internally?
By leveraging this rule we could have a meta-rule that disallow useless properties on an element.
i.e. either 1. the property is invalid (hence ignored) for that element or 2. it's superfluous (it's the default e.g. an anchor is inline)

  1. is fairly easy to implement (it's just tedious)
  2. is impossible (because the property might be set using JS but reset using CSS)

If that's possible then we could have a new property-no-invalid rule for 1. (would be limited to type selectors though).
Id advise to support both blacklisting and whitelisting for the implementation.

@jeddy3
Copy link
Member

jeddy3 commented Dec 18, 2021

@Mouvedia

can we reuse rules internally?

Plugins can.

If that's possible then we could have a new property-no-invalid rule for 1

See this plugin - which was removed from core a while back.

@Mouvedia
Copy link
Contributor

See this plugin - which was removed from core a while back.

@jeddy3 that's not what I am talking about.
The reference, in my case, is the element.
i.e. on that element the property does nothing (irregardless of the other properties)
It wouldn't pose problems unlike that plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add rule-selector-property-disallowed-list
4 participants