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 link to stylelint-selector-tag-no-without-class plugin #3201

Merged
merged 1 commit into from
Mar 9, 2018

Conversation

aboks
Copy link
Contributor

@aboks aboks commented Mar 7, 2018

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

None, this is a small documentation addition.

Is there anything in the PR that needs further explanation?

No, it's self explanatory.

@hudochenkov
Copy link
Member

Congratulations on your first stylelint plugin! :)

I took a quick look at it, and have one suggestion. I think it doesn't need a secondary option. Primary option could have an array. If array isn't empty, then the rule is enabled. You can take any *-whitelist/*-blacklist rule as an example. E. g. function-blacklist.

@aboks
Copy link
Contributor Author

aboks commented Mar 7, 2018

Thanks for the suggestion. That option crossed my mind, but I was unsure whether an array as primary option would make sense when the rule would get (other) secondary options in the future (which I totally see happening). Would something like that be considered good practice? I couldn't find other examples of such situations, and the configuration looks a bit messy:

"rule": [ ["tag1", "tag2", "tag3"], {
    "someOtherOption": true
} ]

@hudochenkov
Copy link
Member

It's ok.

unit-blacklist:

{
	"unit-blacklist": [
		["px", "em", "deg"],
		{
			"ignoreProperties": {
				"px": ["font-size", "/^border/"]
			}
		}
	]
}

Also, every rule could have severity secondary option.

@aboks
Copy link
Contributor Author

aboks commented Mar 8, 2018

Thank you for your feedback. I will aim to release a version 2.0 with the tags as the primary option.

It seems though that https://github.com/stylelint/stylelint-test-rule-tape does not work well with primaryOptionArray for plugins, and thus parses the configuration incorrectly. Are you aware of such a limitation?

@ntwb
Copy link
Member

ntwb commented Mar 9, 2018

@aboks Could you create an issue in that repo documenting what happens and what you expect to happen with some links to or code examples please :)

FYI: stylelint itself pretty much uses Jest for testing these days, so that is more than likely the reason we've not seen this issue before.

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.

None yet

3 participants