-
Notifications
You must be signed in to change notification settings - Fork 34
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
Values! #28
Values! #28
Conversation
Oh and obviously the code in this PR would be broken out into its own repo, but at least right now the tests pass! |
👍 x 1,000,000. |
👍 Sounds pretty good. There is a small thing missing, importing stuff into another name. We need this, to avoid conflicts when importing. I would propose: @import a as newA, b as c from "./file";
or you write the constants in a CSS file and import in js for inline-styles. /* colors.css */
@let highlight: #EF4312; import { highlight } from "./colors.css"; |
You make a really strong case for this. Nice work 👍 |
Naming conventions need work, but this looks pretty good 👍 |
Pretty great, I would agree with @sokra that we need named imports. But other than that I'm pretty happy with this. |
Didn't @geelen already address named imports with this line? @import small as bp-small, large as bp-large from "./breakpoints.css"; |
OH. I totally missed that bit. |
great 👍 |
Yep, that's totally possible too! Though it does raise the question, if I have this: /* colors.css */
@let primary: #BF4040;
.text-primary {
color: primary;
} import styles from "./colors.css"; Should I get a single flat JSON object with import { classes, constants } from "./colors.css"; ICSS |
Oh and if you'd like to talk naming, join the Gitter (https://gitter.im/css-modules/css-modules). I'm by no means fixed on |
Love how this is looking 👍 as @jhewlett has mentioned I wonder if variable reference needs some more syntax around it to separate it from clashing with CSS values. (Eg. defining a constant Also a big +1 to @sokra's suggestion to avoiding naming conflict. The ES6 modules spec has been argued about for an age and has churned out an incredibly well thought out feature-set. I would continue looking to this for inspiration as the module and constant scoping there shares a lot of similarity with CSS Modules. Also really like the idea of being able to import values from CSS into JS! |
Nice proposal! A couple things:
import {classes, constants} from "./file.css"; If constants and classes were combined into a single flat object, iteration over just the classes or just the constants would be difficult. |
The problem with classes and constants being separate is you can't do named imports like this: import { header, content, footer } from './file.css'; It's definitely a more future-proof API, though, so I can see the tradeoff being worth it. |
I prefer the flat namespace, because it can be mapped to ES6 exports. Using CSS Modules like this: import { header, content, footer } from "./file.css";
import * as styles from "./file.css"; may allow automatically detection of unused classes/constants in future. It makes references more "static". Colisions between classes and constants may be a problem, but I think the naming is a bit different and colisions could be easily detected by the compiler. For iteration we could offer exports |
What happens if there's a constant and a class with the same name? @let primary #aabbcc;
.primary {
color: primary;
padding: 4px;
}
import primary from './style.css'
// what is `primary`? I feel like something like the above |
It won't compile. The CSS Modules parser can detect it and throw an error. You must rename it i. e. to |
@geelen I think you can move the postcss plugin into a separate repo, with the infrastructure similar to the other postcss plugins. |
@sokra Good point about static resolution of ES6 imports and potential dead code elimination. With that in mind, I reverse my opinion as that's worth the tradeoff. 👍 |
Best practice in javascript is to export a function or a constructor if you want. @value colors: "./colors.css";
@value primary, secondary from colors;
@value small as bp-small, large as bp-large from "./breakpoints.css"; is like var colors = require("./colors.css")();
var primary = colors.primary;
var secondary = colors.secondary;
var bpSmall = require("./breakpoints.css")().large;
var bpSmall = require("./breakpoints.css")().small; So for very "atomic" css components, like So what do you think about - or if there is a reason to not include something, how to otherwise satisfy this need?
|
Realize I'm 6 months late, but I like the idea of |
And (unless I missed it) it'd be GREAT to have a mention of values on the css-modules github readme. It's a great feature! |
I didn’t read all the comments so please pardon me if this has been asked before, but what would happen when naming a variable (constant, sorry) after an identifier name or a color keyword. Like so: @let red: #ea6153;
.text-primary {
color: red;
} Would it resolve to |
@hugogiraudel currently it gets overwritten. To me that's messy and annoying, so I would prefer to have |
@dimaip |
@sokra good point. Perhaps I should enforce it on our company styleguide. |
I've been kicking around a similar idea: pascalduez/postcss-map#76 |
For me the lack of Consider a simple example: /* ./colors.css */
@value primary: #BF4040;
@value secondary: #1F4F7F; /* ./app.css */
@value primary, secondary from colors; /* ./component.css */
@value primary, secondary from app; I don't want |
EDIT: the final version replaces
@let
and@import
with, simply,@value
. See the postcss-modules-values for more infoHey friends, I've got an implementation of a legit extension to CSS Modules that I think we should adopt, fully, in core, and tell people to use it. I.e. this would become the third of three features (local scoping & composition are the first two), so this is a reasonably big change to our public API.
But I think it's the final piece of the puzzle to make CSS Modules really capable of changing the way people structure their CSS.
I thought I'd start with the syntax I'm proposing. We're totally going to bikeshed these names, but let's do that in a chat room, not in this issue. This issue is for discussing whether this deserves to be part of CSS Modules core.
And with that, my proposal:
ConstantsValuesSo, it breaks down to two things:
@let
to declare a constant. It can be one of the following:"/path/to/file.css"
. The quotes are kept, so when this is used, it has to be somewhere that quotes would make sense (i.e. an@import
orcomposes
statement)#BF4040
orinset 0 0 2px 2px black
screen and (max-width: 599px)
@import
to import a constant.@import theirName as myName
@import a, b from "./c.css"
.@let foo: "./foo.css"; @import bar from foo;
Once you have your constant defined, it'll be substituted in in one of two places:
box-shadow: 0 0 20px secondary
wheresecondary
is a Constant@media small
wheresmall
is a ConstantOne more thing, for Sass compiler compatibility, both at-rules can be prefixed with a single
-
, so@-import
and@-let
will work (@import
will make Sass try to load a file and your computer will set on fire). And the@let x: y;
statement doesn't need the:
in the middle since Sass crashes on that too. I prefer using the:
(I think it reads better) but Sass folks won't be able to insert it.Rationale
First up, this solves some problems people are having: #25 #33 #36
It also completely removes the need for my custom-media PR: #18
It gives the whole system a flexibility we just don't have right now. I mean, none of what we're doing can't be done with other things, like a PostCSS plugin or Sass. But consider the following when you mix & match Sass & CSS Modules:
It works, and I think I'd like to put out a blog post about why you, if you have a big Sass app and want to start using CSS Modules, you should aim to get to here as a stepping-stone, but I think if that's the only way to do anything advanced with CSS Modules it's confusing at best and misleading at worst.
My key complaint is that you're mixing and matching two different ways of including other files. When you use Sass
@import "./vars"
, you're effectively concatenating that file into yours. So if you have any actual CSS markup there (i.e. not just variable definitions) that'll get duplicated through your app. That's because Sass is designed to be a single-global compilation and CSS Modules is a multi-file system. CSS Modules won't inject the same content twice, but if you use Sass@import
it will. I think we can do better.My other argument is that we have an opportunity here. If CSS Modules can handle symbol-passing between files, we can totally break our dependency on global preprocessors. Let's look at the above situation with my Constants proposal:
Now, we're still using Sass (as a lot of people will want to do), but we've made it a single-file preprocessor. We now use CSS Modules to export information (using
@let
), but we've broken the global shackles of Sass. And, in fact, our component file isn't Sass at all! We're starting to actually use modularity to change the level of abstraction that component authors are working against.I believe that by promoting Sass as being the go-to for all the variables, mixins & functions that are missing from CSS Modules is missing a huge opportunity to better inform people about the way they can structure the app. By offering Constants, we can say "sure, use all the Sass you want, but make it compile constants & classes that you can import and compose throughout your application." By breaking the old-school idea of files being concatenated together, we get to truly modularise our code.
This all applies to PostCSS as well, since they almost all assume they're working on a single global css object. I just think that the unit of "single global css object" is now a single file, not the concatenation of many files.
Oh and one more thing, this would allow CSS Modules to
@import
constants from, say, a JSON file that's shared between some inline styles and your whole CSS codebase. Some folks have been asking for that, and this would let us offer it.And just to be clear, I'm proposing adding this and basically no more. Beyond this, everything people want to use can live in userland (probably as PostCSS plugins). But at the moment we have no human-accessible way to pass information between files, and I think we need it.
Thoughts?