postcss-bem-linter icon indicating copy to clipboard operation
postcss-bem-linter copied to clipboard

namespaced custom properties

Open VinSpee opened this issue 9 years ago • 26 comments

I'd love to be able to namespace custom properties, ala:

/** @define Button */

:root {
  --ns-Button-size: 1rem;
}

Is there any way that I'm missing to do this currently?

VinSpee avatar Feb 09 '16 17:02 VinSpee

I agree that whatever namespace is set should apply to custom properties.

@simonsmith, do confirm that SUIT CSS would like that?

davidtheclark avatar Feb 09 '16 17:02 davidtheclark

We don't currently have any formal guidelines for custom properties, although there is a long outstanding issue to get it all written up. I think it makes sense to allow components to have namespaced custom properties, and we can include that in the documentation when I get round to doing it.

cc @timkelty @giuseppeg

simonsmith avatar Feb 09 '16 17:02 simonsmith

Yeah I don't see why we shouldn't allow something like that.

giuseppeg avatar Feb 09 '16 18:02 giuseppeg

Sounds good to me as well.

timkelty avatar Feb 09 '16 19:02 timkelty

Is the idea here to allow it, or to enforce it? That is, if you have defined the namespace ns for your stylesheets, should we enforce that every custom property in a linted stylesheet has an ns prefix? Or should we add this as an option?

davidtheclark avatar Feb 10 '16 03:02 davidtheclark

maybe optional for now, forced in the next major release?

VinSpee avatar Feb 10 '16 04:02 VinSpee

If it is enforced you won't have any choice but to release a major version.

maybe optional for now, forced in the next major release?

+1

In this iteration we should just add support for namespaces and make it fail if the component part is missing otherwise things like --ns-fooBar would be valid – but I guess that this was obvious.

giuseppeg avatar Feb 10 '16 05:02 giuseppeg

If it is enforced you won't have any choice but to release a major version.

Nothing wrong with releasing major versions.

I'd vote that if you have a namespace option set then it should apply to the custom property too.

simonsmith avatar Feb 10 '16 10:02 simonsmith

I don't know what is their release cycle like, often times people prefer to have a milestone and include more than one branch before making a major release.

I don't have any problem with that anyway!

giuseppeg avatar Feb 10 '16 17:02 giuseppeg

Yes, that was my meaning as well

VinSpee avatar Feb 10 '16 17:02 VinSpee

Cool, thanks for piping up. Open to a PR to speed things along.

davidtheclark avatar Feb 14 '16 23:02 davidtheclark

I'm happy to give it a whirl. @davidtheclark I noticed that the only place we're checking for namespace now is in the component checker, which brings a different context than the custom property checker. How can I appropriately pass the namespace into the custom property checker? Any "correct" way that I'm missing?

VinSpee avatar Feb 17 '16 19:02 VinSpee

@simonsmith , @giuseppeg , @timkelty : Should namespaces also be applied to utility classes? I'd expect so, myself, but that's not what's going right now. Unless there's an objection, I'd think we should take this advantage to use the namespace everywhere (component classes, utility classes, custom properties).

davidtheclark avatar Feb 18 '16 01:02 davidtheclark

@VinSpee : The decision whether to apply the same treatment to utility classes will affect what I'd consider the "correct" approach here.

davidtheclark avatar Feb 18 '16 01:02 davidtheclark

I would agree that they should apply to utility classes as well

VinSpee avatar Feb 18 '16 01:02 VinSpee

In my opinion there is no need to add namespace support to utility classes for three reasons:

  • u- is already a namespace
  • you can already add levels to the namespace, e.g. suicss/utils-size uses the u-sm- to namespace the size utilities for small devices
  • utility classes are single purpose and final i.e. they use !important and can't (shouldn't) be overridden. So for example I don't see why you would make .ns-u-posFixed when you already have .u-posFixed.

giuseppeg avatar Feb 18 '16 02:02 giuseppeg

Example: I have a spacing utility as a part of my base framework. It takes variables. I currently would like to have it as

.blank-u-mgBsm

It would also contain variables as --blank-sm

Do ou think this would be better served as a component in this instance? How would that look?

VinSpee avatar Feb 18 '16 02:02 VinSpee

@VinSpee because of the single purpose nature of utilities you won't (or shouldn't) ever have duplicates. This means that you can define .u-mgBsm and a generic variable --u-mgBsm

:root {
  --u-mgBsm: 1em;
}

.u-mgBsm {
  margin: var(--u-mgBsm);
}

and then in your theme/framework override the variable

:root {
  --u-mgBsm: var(--blank-sm);
}

giuseppeg avatar Feb 18 '16 07:02 giuseppeg

@giuseppeg , @VinSpee : What about utilities with more generic names that could be implemented in different ways? Let's say I import a library that has a u-clearfix class, but I don't like the implementation so want to create my own, nm-u-clearfix?

davidtheclark avatar Feb 18 '16 13:02 davidtheclark

I'd be inclined to agree with @giuseppeg in that a utility should generally be unrelated to components. Configuring them with custom properties is rare, but a valid route to go down if they need it.

Let's say I import a library that has a u-clearfix class, but I don't like the implementation so want to create my own, nm-u-clearfix?

@davidtheclark That feels quite edge case. Utilities tend to have a very specific purpose and are often only a few declarations if that. I couldn't imagine there would be a radically different u-cf implementation.

This discussion did come up some time ago on the SUIT repo and I agree with Nicolas' comment at the time:

you should be using utilities for styles that can work in any app

So far I've only seen hypothetical reasons as to why you'd need to namespace them, rather than a real world example. Having used SUIT on a few projects I've still yet to bump into this issue.

Happy to be proven wrong though :)

simonsmith avatar Feb 18 '16 19:02 simonsmith

Ok, well I'm fine with just namespacing custom properties.

davidtheclark avatar Feb 18 '16 19:02 davidtheclark

Sounds good! On that note, what do you think is the "right" way to implement here?

VinSpee avatar Feb 18 '16 19:02 VinSpee

I'm going to defer to @simonsmith and @giuseppeg on namespacing utilities. I could go either way.

u- is already a namespace

If that is part of the argument, I think at the very least we should make it clear in the docs that the u is occupying the namespace column.

If that is the case, if someone really wants non-conflicting utils, might they do something like: .myU-mgBsm? Then I assume we'd have to have the linter have a setting for utilityNamespaces or something that defaulted to ['u'].

timkelty avatar Feb 18 '16 22:02 timkelty

@VinSpee Sorry, I'm going to need some time to look at the code and think about it. If you come up with any proposals, feel free to PR. Otherwise I'll try to get to it this weekend.

davidtheclark avatar Feb 19 '16 15:02 davidtheclark

I think this is the way to add the feature:

  • Extend the library so that the componentName pattern can be a function, like componentSelectors: A single function that accepts a component name and returns a regular expression, e.g. js componentSelectors: function(componentName) { return new RegExp('\\.ns-' + componentName + '(?:-[a-zA-Z]+)?'); }. The pattern to follow here is component selectors.
  • Modify the suit pattern in preset-patterns.js to use that feature.

davidtheclark avatar Feb 20 '16 01:02 davidtheclark

Sorry I think the above may have been incoherent. I guess we need a custom property validating function. I'll look into it now.

davidtheclark avatar Mar 19 '16 23:03 davidtheclark