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

Deprecate root rules #2226

Closed
jeddy3 opened this issue Dec 31, 2016 · 11 comments
Closed

Deprecate root rules #2226

jeddy3 opened this issue Dec 31, 2016 · 11 comments
Labels
status: needs discussion triage needs further discussion

Comments

@jeddy3
Copy link
Member

jeddy3 commented Dec 31, 2016

Continuing to split #2079 into separate issues.

Next up:

  • custom-property-no-outside-root
  • root-no-standard-properties
  • selector-root-no-composition

These three rules have been in since the very beginning. I think it's clear what they do, but I've never really been sure why they would be used.

Am I right in thinking that custom-property-no-outside-root is to ensure compatibility with the following limitation of postcss-custom-properties?:

"It currently just aims to provide a future-proof way of using a limited subset (to :root selector) of the features provided by native CSS custom properties."

Having custom properties outside of :root is valid and a proper use case:

"Custom properties are ordinary properties, so they can be declared on any element..."

So, in this instance, it looks like we have a rule in core to workaround a limitation of a PostCSS transformation plugin. As full support for custom properties continues to be adopted by browsers, this rule feels like it should be a plugin.

I'm also struggling to find anything in the spec about disallowing composition of the :root selector, nor any issue with having standard properties within the :root declaration block. The spec simply says that :root represents the element that is the root of the document e.g. html.

I'd really appreciate someone shedding some light on these three rules.

@jeddy3 jeddy3 added the status: needs discussion triage needs further discussion label Dec 31, 2016
@davidtheclark
Copy link
Contributor

These rules are not to disallow invalid code, but to enforce certain patterns. These rules were adapted from postcss-bem-linter. So at the time, at least, SUIT CSS users were interested in them. I'm not sure if this was only a workaround for the PostCSS plugin or if it was also considered useful. One goal could be to place restrictions on root :root and on custom properties, so :root alone is the site for declarations of custom properties.

@jeddy3
Copy link
Member Author

jeddy3 commented Jan 8, 2017

Thanks. That sheds some light on their origins.

One goal could be to place restrictions on root :root and on custom properties, so :root alone is the site for declarations of custom properties.

That makes sense. I guess it now becomes a question of whether these patterns are idiosyncratic or not.

@simonsmith I hope you don't mind me mentioned you, but we're doing an audit of stylelint's rules in preparation for 8.0.0. There are three rules we're trying to understand:

Are you able to shed any light on why these rules might be used within a SUITCSS context as we believe that to be the origin of them?

@simonsmith
Copy link
Contributor

@jeddy3 Hey,

I think these rules are mainly to work around the limitations imposed by replicating root with a preprocessor but also to ensure styles don't leak out into other components.

custom-property-no-outside-root: I think this works around the postcss-custom-properties limitation that you pointed out already.

root-no-standard-properties: By forcing use of root it means that all components have to share that global space. If they all set color then it would clash. Instead things like --Component-color ensures they don't interfere with each other.

selector-root-no-composition: I'm not too sure on this one. It dates back to when rework was used for linting. I would assume it ties in with the limitation of replicating custom properties without a DOM.

Interestingly custom-property-no-outside-root is not used in the stylelint-config-suitcss package.

Out of interest here's what GitHub search yields (under the code tab):

root-no-standard-properties: 760
selector-root-no-composition: 816
root-no-standard-properties: 578

I'm happy to have them moved into a plugin if you'd prefer to remove them from the core.

@jeddy3
Copy link
Member Author

jeddy3 commented Jan 10, 2017

@simonsmith Thanks for replying! That continues to shed some more light on things.

if you'd prefer to remove them from the core.

I think so. They feel quite idiosyncratic and I think better suited as plugins. @stylelint/core Any objections?

@davidtheclark
Copy link
Contributor

Sure

@jeddy3
Copy link
Member Author

jeddy3 commented Jan 10, 2017

Tracking in #2123

@jeddy3 jeddy3 closed this as completed Jan 10, 2017
@jeddy3 jeddy3 mentioned this issue Jan 10, 2017
13 tasks
@simonsmith
Copy link
Contributor

Where will these rules be moved to? A new npm package?

@jeddy3
Copy link
Member Author

jeddy3 commented Jan 10, 2017

They'll be deprecated in an upcoming release. The deprecation warning will have a link to http://stylelint.io/user-guide/release-planning/ which will (once this PR is in) ask for the community's help.

The community will then have time, and if there is the desire, to convert these rules to plugins before they are removed outright in 8.0.0. We took the same approach with 7.0.0 and it worked well.

More background about this can be found in #2079. In short, it's to keep development of stylelint sustainable by creating and sticking to a set of criteria around what should be a rule and what should be a plugin.

@simonsmith
Copy link
Contributor

I see.

Well there is desire to continue using this rules in SUIT CSS. Perhaps when you get round to removing the root ones I could put together a root rules plugin.

@jeddy3
Copy link
Member Author

jeddy3 commented Jan 11, 2017

Perhaps when you get round to removing the root ones I could put together a root rules plugin.

That would be super.

and if there is the desire...

I should have been a bit clearer :) I meant that in the context of browser support for custom properties being on the horizon, which might make preprocessing the custom properties with SuitCSS unnecessary and, perhaps, these rules redundant. This might not be the case, though. I guess it depends on how things like theming might work when browsers support custom properties.

@simonsmith
Copy link
Contributor

simonsmith commented Jan 11, 2017

I expect IE will hold it back for some time :)

But yes, would be ideal to not need them at all!

davidtheclark pushed a commit that referenced this issue Mar 15, 2017
* Add stylelint-suitcss to plugin list

Also updates deprecation notices in READMEs and code

Addresses #2226

* Add a period

* Copy edits
alexander-akait pushed a commit that referenced this issue Mar 21, 2017
* Add stylelint-suitcss to plugin list

Also updates deprecation notices in READMEs and code

Addresses #2226

* Add a period

* Copy edits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs discussion triage needs further discussion
Development

No branches or pull requests

3 participants