-
Notifications
You must be signed in to change notification settings - Fork 558
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
How to use composition in elements without className? #33
Comments
Btw, I forgot to mention that @nkbt suggested before the use the |
I believe that styling html tags is unsafe and goes against css-modules
core concept. Since if you have nested components (which you always have),
you get into old good "inheritance" and specificity issues.
Maybe this could be also automated somehow, but not sure, since we cannot
manipulate rendered html.
|
@nkbt I hear you and as I told you before I don't mind at all 😊 after start using React I drop the html tags styling. It's just because I use markdown a lot and I can see other cases where I'd use a loader e.g. YAML which will return a "pure" HTML and I'd have again to overwrite the loader and parse it. Just wondering if we could achieve or even hack something around only for this cases. |
By the way that sounds like a good idea to PR yaml-loader to add classes to each element ;) <h1 class="h1"> |
@nkbt yup! Nice one! |
oh sorry that would not work for obvious reasons.. disregard previous comment. |
@nkbt 👍 going to wait for Glen to see if he has any suggestions if not I'm happy to close this issue. |
Ok, so. Yes. This is a problem. Let me give you some background (maybe you're already across this but for other readers of this issue):
.foo {
color: red;
}
.bar {
composes: foo;
background-color: blue;
} import styles from "./foo.css";
return "<p class='${styles.bar}'>Content</p>" You get the following ICSS: :export {
foo: foo_98812cadf;
bar: foo_98812cadf bar_312cbca63;
}
.foo_98812cadf {
color: red;
}
.bar_312cbca63 {
background-color: blue;
} So, your HTML renders out as So if you don't have control over the markup being injected then composition doesn't work. I actually faced this exact problem and wrote a monstrous JSPM loader for it. It parses the markdown as YAML (for frontmatter), then markdown then JSX, then returns an ES6 function that accepts a It gets used here and lets me do cool stuff with inline JSX in markdown: But I digress, back to the issue at hand. Theoretically, you could write a runtime JS component that gave you this more advanced behaviour, where it watches the live DOM and attaches classes based on these more advanced rules. That could be kind of neat, but at this stage it's not something I'm going to work on. For one, I am trying to avoid using descendent selectors & bare tags as much as possible, except for contextual overrides that don't care about the nature of the things they're overriding. E.g: .MyComponent {
> * {
margin-top: 1rem;
}
> :first-child {
margin-top: 0;
}
} Secondly, if you add a runtime dependency you need to be damn careful not to break server-side rendering. Because all the React SSR stuff already evaluates the render tree, injecting classnames there works really nicely. And since we're using plain-old-CSS, we get pseudo-selectors like Probably should also point out that the following usages of .foo .bar {
composes: different-when-nested;
} @media (max-width: 599px) {
.bar {
composes: something-on-small-breakpoint;
}
} The media query one, at least, has a decent fallback: /* A class can use media queries then get composed */
.headline {
font-size: 4rem;
}
@media (max-width: 599px) {
.headline {
font-size: 3rem;
}
}
/* You can use suffixes like Tachyons/Basscss */
.foo {
color: red;
}
@media (max-width: 599px) {
.foo--small {
color: red;
}
}
.bar {
composes: headline foo--small;
} The thing I like about this is that we're building our own abstractions on top of the basic CSS mechanisms. I see |
@geelen Adding dynamic classes to default elements such as |
Thanks @geelen for the background, for spending time writing this.
I think this is one of the problems I face right now building a site, not an app or component.
Couldn't agree more. CSS modules is all about simplicity.
Ok that's interesting thanks for sharing. I think a variation of your markdown.js could be used as markdownLoader on webback if you're using CSS modules.
😂😂😂
I agree! I think is worth to apply this in real projects. Right now I have the feeling that CSS modules is perfect for components and self-contained apps, but for page layout/design I think we are still missing some pieces. I know @markdalgleish made react-themeable to address some of the limitations but this isn't an issue with theming. Maybe we need something else in between 😊. I'm happy with the answer, please feel free to close this issue. Cheers 🍺 |
@NekR I'm afraid it's simply not possible to use But, while I'm finding plenty of uses for Oh, that and it's all just PostCSS transforms so if you want to go crazy and write a stage that detects if |
By the way we never used `composes` and used only simple 1 level deep
classes in our css.
Composition was done with react components itself. If something looks same
- it is a component that can be reused in many situations.
You can even go more crazy doing simple component-wrapper, the only thing
that it would do - make text color `red` on passed child (via classname),
for example. This kind of comletely eliminates the need in css-modules
`composes`
I am keen to see how it will go on a scale though.
|
I do not propose to use :global(body) {
margin: 0;
/ * ... */
add/merge/whatever: text-color from './colors.css';
} which would produce: body {
margin: 0;
/ * ... */
color: #333;
} but not any magic from
I did not say that it should be still called |
Hey @NekR, I actually really like this idea. One of the immediate questions people ask about CSS Modules is "how can I do variables?" In fact, someone trying to get a variable-passing syntax into CSS Modules was why we broke out ICSS — so people would be able to write their new ideas in userspace and we'd bring modules into core if we really felt they were globally useful. I'm going through that process with my own stuff, too: css-modules/css-modules-loader-core#18 So we're hoping to tread the line about being defensive about adopting ideas but enthusiastic about supporting them. But we're all new to this, so I apologise if I came across as being defensive. For me, there are two competing ideas at play. The first is that good design should guide you to good practice. I see using tag selectors and nesting as not good CSS practice, so that makes me reluctant to include anything designed directly for them. The second is that CSS Modules should maintain as much as possible of CSS. For every rule we make, we know there'll be exceptions. If we disable any parts of CSS without giving a better alternative, that's shitty. That's why there's Your idea is somewhat positive in both respects. Being able to reuse all your modules in all your styles (regardless if they are modules themselves) is definitely a win, despite wanting to avoid tag selectors where possible. And having So I could go either way, and I'll invite folks to leave their comments here either for/against this idea. The implementation would be tricky, and doesn't fit well with the way ICSS works, but the question is should we support something like composition from tag selectors? |
Thanks for the response @geelen. I understand your position and that you do not want to bloat CSS Modules with every feature requested or made it another Sass/Less/Stylus/etc. I actually did not mean allowing cascade selectors (hah) like this So it indeed might be a good idea to allow new keyword :global(.my-class-for-body) {
margin: 0;
/ * ... */
merge: text-color from './colors.css';
} which will produce: .my-class-for-body {
margin: 0;
/ * ... */
color: #333;
} So in this case we still have restriction to compose/merge only classes/into classes, but will ability to export merged rules to global scope.
Yep, feedback and discussion would be good here. Maybe other people have better ideas :-) |
Isn't it something, that less/sass/etc already do? I thought if I want
styles merging, I would use less for that and then pipe it to css-modules.
Or I miss some important difference?
|
That' exactly what I would propose too. CSS Modules is designed is a way, that allows to use less/sass/stylus/postcss on top of it. So you get all the fancy features of css preprocessors while still working on a modulized CSS file. This way you can import your variables/mixins. I. e. with webpack (less.js is currently a bit buggy, because it rewrites |
This looks like to have "modules" I need to have too much files, isn't? // vars.css; postcss vars
$text_color = #333;
$brand_clor = red; // colors.css; css module
@import './vars.css'
.brand {
color: $brand_color;
}
.text {
color: $text_color;
} // main.css, simple file
@import './vars.css'
body {
/* ... */
color: $text_color;
} // my-component.css, css module
.button {
compose: brand from './colors.css'
} This is really confusing. As per your blog posts you are positioning CSS Modules as a replace for less/sass/whatever preprocessor, and now you are saying that it's not replacement but a some piece like any other Postcss plugin. If so, then I believe CSS Modules is already done at this point since it cannot bring anything new to the world here what Postcss can't. |
I wouldn't say so. I would say you now have it nicely separated.
less/sass/whatever are tools that make it easier to write CSS. CSS Modules is a technology which make it possible to write modulized CSS. CSS Modules may make less/sass/whatever unnecessary, because your CSS files are just so simple that you don't need tools that make writing of them easier... But there is no need to style the body tag in your case. Just use a /* text.css */
.normal {
font-family: Arial;
color: #333;
}
.brand {
composes: normal;
color: red;
font-weight: bold;
} /* Application.css */
.context {
composes: normalText from "./text.css";
} /* BrandButton.css */
.button {
composes: button from "./Button.css";
composes: brand from "./text.css";
} /* Application.js */
import styles from "./Application.css";
class Application extends React.Component {
render() {
return <div className={styles.context}>
{this.props.children}
</div>;
}
} |
Hmm.. Maybe I should decide by myself to style body or not. If even use
React or not. Is CSS Modules only for it?
Again, you are positioning CSS Modules as a replacement to preprocessors to
get people around here and then saying "this is not we", " do it yourself",
"you do not need it". Really, I understand that your are not saying to me
and others that " you should CSS Modules", but yor are pointing it as a
"panacea", " rescue of the web and CSS", "best practice".
This all seems just a strange to me.
|
Oh, and about CSS vars.. Remember that in some blog post you (team) pointed
that using vats in files is not sufficient and it does not scale fine. And
your saying that in previous case I have "nice separation".
So if CSS Modules is only about Modules, shouldn't you lock all the issues
and fix only bugs? Because this seems like this tools already does its job
and hence should not be extended further.
|
@NekR CSS-modules is a panacea from CSS inheritance/specificity/globality decease. It gives you a tool to write small, clean, isolated styled modules. It is completely up to you if you want to use variables, imports and other workarounds from old css world - you still are able to use sass for that. And css-modules give you a great opportunity to mix and match tools. It is never meant to be a replacement for sass. It is just a completely new way of styling things. It is up to you if you want to replace your styling techniques with this new way, or not. Many people do not replace but add. We replaced completely in our projects and can't be happier with new way of styling components for past 2 months. PS: please, try to be a little more humble and lower your tone, it starts to feel sort of harsh. Cheers |
You know, my tone it exactly how it wanted it to because I am extremely disappointed with your answers here.
I understand what CSS Modules is great tool and how it can help. |
Hey @NekR, I really don't appreciate your tone (what is it with this particular topic and people behaving like jerks?). You had a chance to propose something, I spent considerable time explaining the constraints of the system and the relative merits of your approach, and we landed on a potential implementation and both agreed to open it up for feedback. Now, when that feedback indicates that what you want is already possible without needing to invent new syntax, you're now "extremely disappointed"? Were you trying to find a solution or trying to win an argument? Because only one of those is acceptable behaviour on this issue tracker. CSS Modules is not designed to replicate Sass, it's designed to hugely reduce the need for it. It offers a solution to a problem (global names in CSS) and a powerful new technique (composition) for styling reuse. There's a huge bunch of us trying to figure out what more if anything we really need, and the point of issue threads like this is to find that out. So to answer the original question, and given the way composes works (as I described at the top), the proposed solutions are the idea of using a However, I do think this is one of the most difficult aspects of CSS Modules for newcomers to understand, so I'm going to raise another issue around adding that to the documentation. |
@geelen the information you shared here is really valuable. Maybe add a "constraints" section with some of these information and maybe another section like "CSS Modules is NOT for..." This would help set expectations for newcomers |
@geelen I'm following your suggestion and creating a healthy issue to discuss this.
What do you recommend for the follow scenario:
I'm re-writing the page/docs for Minigrid and I'm using
markdown-loader
for generate content for both. I can use my own markdownLoader to parse the text, add the classNames and give it back to the webpack. For this simple case it works fine, however my concern is when the project grows this could become problematic. I'd like to be able to achieve this without have to overwrite the loader and parse the content.Thoughts?
Thanks 🍺
The text was updated successfully, but these errors were encountered: