-
Notifications
You must be signed in to change notification settings - Fork 907
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
SFC style ordering #808
Comments
Hi @Justineo I understand your problem, but I'm afraid the proposed solution will be next to impossible to realize. You see, in order to be able to inject styles of the parent components before the styles of the child, vule-loader would have to read the component's code:
|
I think the logic is not that complicated. Just ensure we import style of current component after all other // AlertButton.js
import MyButton from './MyButton.vue' // 'MyButton.css' is imported inside this
import './AlertButton.css' // Insert styles of current component after all other imports
export default {
components: {
MyButton
}
} It doesn't really matter whether we are actually "extending" components. It's just keeping the way CSS handles imported styles: always import other styles first ( |
This would indeed be difficult because:
Still I think relying on CSS appearing order is probably not that good an idea in the first place... |
If we do not rely on order, we'll have to increase specificity to override styles. I don't think that's a good idea either... |
And the breaking change is not required. We can provide a new option if possible. I understand that the implementation is not trivial in current design but still think the order is important if we don't want to increase specificity or use inline styles. |
Well, I also think it's really difficult but I have tried some different ways to do this and probably found a solution, that is:
So the CSS order is finally right. To be continued, maybe we could:
Any ideas of that? Thanks. |
@Jinjiang I think there needs to be a little more discussion on what exactly the preferred CSS order is. Here is the current state of things: <!-- MyButton component -->
<template>
<button :class='$style.container'>
<slot/>
</button>
</template>
<style module>
.container {
width: 100%;
}
</style> <!-- JoinPage component -->
<template>
<div :class='$style.container'>
<MyButton :class='$style.btn'>
Join
</MyButton>
</div>
</template>
<style module>
.container {
width: 600px;
margin: 0 auto;
}
.btn {
width: 80%;
}
</style> <!-- LoginPage component -->
<template>
<div :class='$style.container'>
<MyButton :class='$style.btn'>
Login
</MyButton>
</div>
</template>
<style module>
.container {
width: 500px;
margin: 0 auto;
}
.btn {
width: 50%;
}
</style> <!-- Root component -->
<template>
<div>
<LoginPage :class='$style.login'/>
<JoinPage :class='$style.join'/>
</div>
</template>
<style module>
.login {
margin-top: 10px;
}
.join {
margin-top: 8px;
}
</style> /* output */
.Root_login {
margin-top: 10px; /* wont work because overridden below */
}
.Root_join {
margin-top: 8px; /* wont work because overridden below */
}
.LoginPage_container {
width: 500px;
margin: 0 auto;
}
.LoginPage_btn {
width: 50%; /* wont work because overridden below */
}
.MyButton_container {
width: 100%;
}
.JoinPage_container {
width: 600px;
margin: 0 auto;
}
.JoinPage_btn {
width: 80%; /* will work because comes after */
} Of course you could solve this problem by always using more specificity in the parent but this is not an ideal pattern for several reasons:
So if we could use order to solve these problems what is the ideal order? From what I can see, the order of the css classes currently is collected as follows: Root I think ideally the order should be: Root So we are still traversing the render tree the same way but the difference is instead of appending classes, prepend and if you come across a component that you have already seen move it to the beginning of the list. /* output */
.MyButton_container {
width: 100%;
}
.JoinPage_container {
width: 600px;
margin: 0 auto;
}
.JoinPage_btn {
width: 80%; /* still works */
}
.LoginPage_container {
width: 500px;
margin: 0 auto;
}
.LoginPage_btn {
width: 50%; /* fixed */
}
.Root_login {
margin-top: 10px; /* fixed */
}
.Root_join {
margin-top: 8px; /* fixed */
} I can see one problem with this strategy which is If you add a scenario where componentA is parent of componentB in one case and componentB is parent of componentA in another part of the render tree but it's hard for me to think of an example like that. What do you guys think? |
Well, I think generally speaking, the behaviour should be consistent between dev (using style-loader for HMR & faster builds) and producton (extracting CSS into one file). And since I believe we can't do much about the order that ExtractTextPlugin creates, I think the best thing would be to stick to that. However, there are other things to consider as well:
So my opinion is pretty much this:
|
@LinusBorg I see your point and I think the most convincing argument is the route code splitting issue. I'm officially convinced you can't rely on css order output. So let's change the discussion to formalizing how you add classes to children components. I agree that it must be done with specificity and I would go even further to say that it's recommended that you always add the specificity even in the case that it's not needed to avoid bugs that can creep in by changing where a component appears (predictability over efficiency). It would be great if there was a Vue community standardized pattern for passing classes to children. There are two ways of doing this. First, via the standard <template>
<child
:class='$style.overrideContainer'
:innerClass='$style.overrideInnerElement' <!-- a prop defined by the child component -->
data-child-styles <!-- attribute to help with specifity -->
/>
</template>
<style module>
.overrideContainer[data-child-styles] {
}
[data-child-styles] .overrideInner {
}
</style>
<!-- or with sass -->
<style lang="scss" module>
[data-child-styles] {
&.overrideContainer {
}
.overrideInner {
}
}
</style> |
@trainiac yes I realized my test doesn't work correctly in all scenarios (which is including your case and SSR / hot reload etc.). I will hold it on for a while. @LinusBorg actually I definitely wish to make it work because imo the current status is unnatural. People write CSS as usual but get the unexpected order results. And always raising up specificities in sub components will make a large project hard to maintain. 🤔 But if it's hard to do. I suggest to take a note in docs about this. And the standardized pattern I suggest is putting global styles in one file and using Thanks. |
Hi all, I just got dispatched here as I have the same problem. let the dev choose the order, like so: <style order="1"> </style>I don't know how styles are collected but if it's possible to reorder them this way, this would allow for total control over how styles are being applied. |
I dont want to speak for @LinusBorg but I think what he is trying to say is even if you had complete control over the order styles, there are scenarios where chunks of your code will not be loaded in the same order. Imagine the hot module reload (HMR) case and the Route code splitting case.
Assuming you aren't using HMR or route code splitting, this can be solved. I think the solution that I proposed above is actually the most natural way to order the styles. I think there are a number of things in the way of being able to do that but I believe it should be the default experience. For users you dont use HMR or Route code splitting it would be a much more intuitive experience. However, if you use HMR or Route code splitting you will still have to deal with load order but perhaps that problem could be solved later down the road. It might be possible for vue-style-loader (to address HMR) and vue-server-renderer plugins (to address Route code splitting) to inject styles into the page in a predictable order. |
ok right... so just throwing ideas: imagine we can order things the way we want in the loader (with the idea I proposed for example), then it's "just" a matter of how HMR and the Route code splitting append the styles in the dom, isn't it ? |
I'm saying why even specify an order at all? Have you read my suggested styles ordering algorithm above? If all of your components had their styles ordered that way you wouldn't need to explain the order to your build tool it would just do the right thing for everyone. @LinusBorg I took a crack at code splitting css files awhile back, but I wasn't actually able to get it work and according to webpack it's actually not a feature currently. Is this what you were referring to when talking about route code splitting for your styles? If so, how do you do it today? |
Nevermind I see that PR is about generating a CSS file per split point and today it inlines CSS with the split js file. Pretty cool 👍 |
I use vue-material throughout a project, the buttons come with an annoying I've ran into this a couple times with different projects, you don't find out until it's near production, which is traumatizing. Is the current guidance to
|
@rayfoss First off, I totally agree that this aspect of how styles are generated is a Major issue. The fact that you can't detect that you have a CSS specificity bug until production runtime is the stuff of nightmares. I'm working on a big application and am working out some guidelines that I'd be happy to share here but I think there might not be a silver bullet. There are a number of application decisions that determine the how one has to address specificity:
I don't think any of these decisions individually are a definite no-no. I think there needs to be a document written up that is working body of knowledge of what the implications of these decisions are with regards to CSS specificity and how one might work around them. I'd like to see if someone has already written it, if not maybe I'll take a crack. |
I have an easy solution that works. I know people know this already, just adding it as it solves my immediate issue. As we always have an id of app on the main component, so prefixing the css with Something like this: #app .login-btn {
// some css here
} This can get repetitive very easily. I mostly use SCSS/SASS so I do something like: #app {
.login-btn {
}
} Now we don't need to remember to add Update |
To clarify, Singhaniya isn't suggesting to add #app to all non-reusable
css, but instead use it when you encounter a specificity glitch.
In my mind, I would expect the following ordering:
1. App
1. App - header
1. App - body1
1. App - body1 - x
1. App body2
1. App body2 - x
1. App footer
Same order should be preserved on dev server as well as production build
and tests should be written to ensure this is the case. Scoping helps, and
it might be the solution to forcing order (infinite hotel style), but it is
not a replacement for predicable ordering.
If no solution can be found with current technology, I might suggest adding
an option to ban standard css and using js driven inline styles for
everything.
On Sat, Jan 6, 2018, 1:22 PM Ankit Singhaniya ***@***.***> wrote:
I have an easy solution that works. I know people know this already, just
adding it as it solves my immediate issue. As we always have an id of app
on the main component, so prefixing the css with #app will give them a
higher priority.
Something like this:
#app .login-btn {
// some css here
}
This can get repetitive very easily. I mostly use SCSS/SASS so I do
something like:
#app {
.login-btn {
}
}
Now we don't need to remember to add #app every time. In fact this is
kind of progressive as vue-loaders comes up with a solution, it will just
be a matter of removing the top nesting.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#808 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA1eLDurmkMDiKamu1YSo4AurOdPiO4Rks5tH8f2gaJpZM4NU_KU>
.
--
*Ray Foss*
|
@rayfoss I'm not sure if the specificity thing can be done on a case by case basis. This will most importantly mean I do some kind of manual testing to identify the problem first, which will be very nasty. I also completely agree that there is nothing better than predictable ordering. 👍 On using inline CSS, this is a controversy which we would not want to get into. Yes, we can do that, but Vue single file component provides a style section which feels elegant to me, but doing inline is also fine if someone prefers. |
Of course I was not suggesting using inline css dev side. I meant adding a
build step that takes our case and applies it inline, without adding a
style tag to the head. Other than perhaps a rudimentary normalizer. This
would give Vue full control of order.
In the meantime, as long as we never ever use ID's, we could leave the task
of ordering to vue-loader by having an auto-scope option that that uses IDs
to assign order. With infinite hotel logic you wouldn't have to know order
ahead of time.
|
@ankitsinghaniyaz I think the #app solution is an elegant way to address most of the problem, thank you for sharing. What if you add a specific attribute to the app container element specifically for this purpose (e.g. [x])? This would de-couple the app id from it's other uses? I think for vue-loader to solve this problem would be extremely difficult given the points made above. I think 99% of problems can be addressed if your app follows this convention. If you are applying a class to a child component always prefix it with #app (or some attribute) Pros
Cons
Examples <template>
<!-- modules example -->
<div :class='$style.container'>
<child-component :class='$style.someClass' :innerClass='$style.anotherClass' />
</div>
</template>
<style lang="scss" module>
.container {
/*styles*/
}
:global(#app) {
.someClass {
/*styles*/
}
.anotherClass {
/*styles*/
}
}
</style> <template>
<child-component class='someClass' />
</template>
<style scoped>
.container {
/*styles*/
}
#app .someClass {
/*styles*/
}
</style> |
We should perhaps change the label from improvement to "Regression" as CSS order between production and dev environments used to not be a problem when CSS was used more directly. I'm now having issues with Vue material base css, theme css, my own global css and component CSS moving around between Dev and Production. The #app trick won't work on CSS that I don't control. Anyone know how to nudge the order of imported (vue material) css above to be loaded above template CSS (global and component)? Update: I just imported it with app.vue instead of main.js, which allowed me to better control the order. |
Small PSA: we will revert to the old behaviour in vue-loader 15, which is already in beta and will be release shortly. |
Id's have really high specificity... why not have loader enclose all scoped css in a random ID of the root? btw, im using "^15.2.4" now my vue-material sass is below my scoped component styles, causing them to take priority. |
- move the main css initialize to app.vue (as suggested on vuejs/vue-loader#808)
@LinusBorg, when you say the old behaviour will be reverted in vue-loader 15, it means the initial issue reported by OP won't happen anymore? And this revert was done in vue-loader 15? I saw you said it was reverted, but I'm asking because I use vue-loader 15 and I still have an issue which seems to me identical to OP's... |
The Code that caused this issue was reverted, yes. Your issue likely has some other cause. |
I'm a little confused at the logic here, and I'm worried the reversion broke a lot of my styles but I'm not sure. Not long ago when I added a global style to a child component, I knew its styles would override the parent's styles even if they had the same specificity because the <style> object would come after. This makes sense because children come after parents. But now all of a sudden my parent styles are sometimes overriding my child styles because the <style>s are all out of order. Can someone explain what the issue was with child styles coming after parent styles and overriding them? That makes so much more sense. |
Just for documentions sake, I fought the same issue @mattaningram is describing above. I don't think it was exactly related to vue-loader, but upgrading from webpack 3 to 4 (and really upgrading from extract text plugin to mini css extract plugin). In my webpack entry point, I was importing some css like this:
But I was also lazy loading my routes like this:
I'm not totally sure, but I think it's due to the way mini css extract plugin splits CSS by default, before extract text plugin would put all CSS in this entry point into one giant CSS file. Now it splits all the imports into one big file, but splits each dynamic import into it's own CSS chunk. In my case the fix was pretty simple: Basically we're telling webpack to lazy load that CSS import, just like we're lazy loading the component's CSS. I think you could also make mini css extract plugin use a single CSS file for the entire entry point like this: https://webpack.js.org/plugins/mini-css-extract-plugin/#extracting-all-css-in-a-single-file Also worth noting this issue only appeared in a production environment, which makes sense since in dev mode vue-style-loader does CSS stuff, not mini css plugin. To debug that I set VueLoaderPlugin to production mode like this:
Docs here: https://vue-loader.vuejs.org/options.html#productionmode I know this isn't exactly what this issue is regarding, but this was the closest thing I could find when googling so I wanted to document my solution for others. |
Coming from nuxt/nuxt#4219 and it seems the issue still exists. ;-( The only solution that works for me is #808 (comment) Any suggestions appreciated. |
What problem does this feature solve?
Let's say we have two components:
We've made a wrapper component called
AlertButton
to "extend" the originalMyButton
component and we intend to make its text color red. But as a result the style ofAlertButton
comes earlier thanMyButton
now using vue-loader.IMO styles of "parent" components should have higher precedence than their children (or "superclass" in inheritance), just like that when we compose a webpage with HTML elements, whose default styles comes from UA styles and have lower precedence than author styles (the styles of the "parent component"). So I think the order of the result CSS should be MyButton - AlertButton instead of AlertButton - MyButton (which is the current behavior of vue-loader).
What does the proposed API look like?
Not proposing API update yet.
The text was updated successfully, but these errors were encountered: