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

fix ordering of css dependencies #109

Open
ianstormtaylor opened this issue Jul 1, 2013 · 14 comments
Open

fix ordering of css dependencies #109

ianstormtaylor opened this issue Jul 1, 2013 · 14 comments
Labels

Comments

@ianstormtaylor
Copy link
Contributor

right now, the ignoring that the builder does, while preventing dupes, doesn't result in the proper css output. for example, imagine a situation with a parent and two children...

theme/component.json:

{
  "name": "theme",
  "local": [
    "one",
    "two"
  ]
}

theme/one/component.json:

{
  "name": "one",
  "local": [
    "two"
  ],
  "styles": [
    "index.css"
  ]
}

theme/one/index.css:

.one {}

theme/two/component.json:

{
  "name": "two",
  "styles": [
    "index.css"
  ]
}

theme/two/index.css:

.two {}

when resolving the dependencies, the builder should run into theme's two dependencies, and start with one. while evaluating one it should see that it depends on two and evaluate two first. the final output in build/build.css should be:

.two {}
.one {}

since one depends on two. instead, because two gets ignored when evaluating theme, the actual output is:

.one {}
.two {}

and then styles in one that should have overridden styles in two actually need !important flags to work. this makes it impossible (or annoying) to build up css dependencies, even though it would be a badass and obvious way to structure the css deps in themes

@tj
Copy link
Contributor

tj commented Jul 1, 2013

hmm yeah that's definitely not ideal

@ianstormtaylor
Copy link
Contributor Author

running into this more and more as i build out the segment.io theme - but i have no idea how to wrap my head around the recursion to fix it haha, but it will be badass once this works

@mnichols
Copy link

This just bit me on a large project too. Before manually stitching files to workaround I am going to have a look to see if I can patch

@jonathanong
Copy link
Contributor

why even bother referencing locals in non-boot components when they're just CSS? just order them correctly in your boot component's local and you'll be fine.

@tj
Copy link
Contributor

tj commented Nov 27, 2013

@jonathanong it's still pretty critical for some unfortunately since css is kinda lame haha, even simple stuff like normalization libs typically need to be first, or if you want to override something in a dependency then it has to be after. YAY CSS

@ianstormtaylor
Copy link
Contributor Author

CSS, oh baby you!!
You don't got what I need
But you say your just a language
But you say your just a language
Oh CSS!!!

@tj
Copy link
Contributor

tj commented Nov 27, 2013

hahaha nice song ian

@jonathanong
Copy link
Contributor

gah now i'm getting this issue. time to spend time fixing component!

@micky2be
Copy link
Contributor

Never run into this issue yet, but I barely override styles

@ianstormtaylor
Copy link
Contributor Author

yeah this one is the one limiting issue for handling all the native types properly. after this and versioning are handled i'm pretty sure i have no "big picture" issues with component and everything is gravy after that

@marcusandre
Copy link

that would be really nice. i am building a project right now in which i have currently about 20-25 modules in my lib folder. almost all of them have some kind of styling. styles that should always appear "on the top" of the css, i simply put inside a ./lib/layout/[layout.css] which is the first dependency of ./lib/boot - BEM helps a lot here because the order (of the modules) in the compiled css is not that crucial. so it's not only a component(1) problem.

@necolas
Copy link

necolas commented Feb 17, 2014

Is this still a problem in 0.12?

@peteschaffner
Copy link

@necolas yup still a problem in 0.12 -_-

@peteschaffner
Copy link

@jonathanong's flatten.js gets this right btw

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants