Skip to content
This repository has been archived by the owner on Dec 30, 2021. It is now read-only.

Custom CSS source files as SCSS #12

Closed
chrismou opened this issue Jun 7, 2016 · 11 comments
Closed

Custom CSS source files as SCSS #12

chrismou opened this issue Jun 7, 2016 · 11 comments
Assignees

Comments

@chrismou
Copy link
Member

chrismou commented Jun 7, 2016

Set it up so that any custom CSS can be written in SCSS.

Will require adding something like to sass watch as part of the of the npm run watch process, and sass build as part of the distribution build process.

Obvious benefits are tidier CSS, mixins and more reusable styles.

Once done, fold the existing stylesheets (ie, miniplayer.css) into the build

@chrismou
Copy link
Member Author

chrismou commented Jun 7, 2016

I may take this one. Only ever used sass with grunt, am interested to know how different it is to set up with gulp

@chrismou chrismou self-assigned this Jun 7, 2016
@chrismou
Copy link
Member Author

chrismou commented Jun 7, 2016

@jacobwgillespie There looks to be some degree of sass building already. ie, miniplayer.scss. Is this an internal electron process? And where are these actually built? They don't show in the app folder 😕

I've done the gulp stuff - was fairly simple - and I'm wondering if I can get away with folding the existing scss files into this build process so we just have a single minified stylesheet in the distribution

@jacobwgillespie
Copy link
Member

jacobwgillespie commented Jun 7, 2016

Themes should likely be set up as another gulp task like you mentioned... With the react components, they have CSS modules based SCSS stylesheets (for instance see this example) that are compiled by webpack, but that's mainly for those components, and themes wouldn't make sense as part of that build pipeline.

Or potentially if we want access to the theme SCSS inside the JS as raw CSS, we could set up another loader config inside of webpack to specifically compile and load those files, but that could always be added as an enhancement later after we get basic building operational.

@chrismou
Copy link
Member Author

chrismou commented Jun 7, 2016

I'm not thinking so much with themes, I imagine that being separate (but yeah, using sass) - this is more for any styling we do for core stuff. miniplayer, last.fm button, settings menu, etc

@jacobwgillespie
Copy link
Member

jacobwgillespie commented Jun 7, 2016

Oh, that build pipeline is already set up - it's building with CSS modules in addition to SCSS, so essentially you import the file from JS and it gets automatically included in the build. Classnames are automatically mangled / minified, so you do something like the following:

// Component.js
import styles from './Component.scss';

export default function Component() {
  return <div className={styles.myCSSClass}>test</div>;
}
// Component.scss
.myCSSClass {
  color: red;
}

And that JS variable styles will contain a mapping of original class names to new names.

If you need "global" rules (disabling the minification), you wrap it in a :global block, like so:

:global {
  .preserved-class-name { color: red; }
}

As long as that file is included in JS someplace, it will be in the eventual build.

If you want to include a CSS file but do nothing with the classes, then just:

import './Styles.css'

@jacobwgillespie
Copy link
Member

There's more info on all this if you Google for "css modules".

@jacobwgillespie
Copy link
Member

One other note: you won't see the extracted CSS files in the build directory unless you're building for production / distribution. In dev the CSS will be contained inside the JS files.

This is the code that enables / disables that extraction:

const applyExtractText = (configToExtend) => {
configToExtend.module.loaders.filter((loader) =>
loader.loaders && loader.loaders.find((name) => /css/.test(name.split('?')[0]))
).forEach((loader) => {
const [first, ...rest] = loader.loaders;
/* eslint-disable no-param-reassign */
loader.loader = ExtractTextPlugin.extract(first, rest.join('!'));
delete loader.loaders;
/* eslint-enable no-param-reassign */
});
configToExtend.plugins.push(
new ExtractTextPlugin('[name].css', {
allChunks: true,
})
);
};
if (!DEV) {
applyExtractText(config);
}

@chrismou
Copy link
Member Author

chrismou commented Jun 7, 2016

Interesting. Though I'm not sure I like the idea of referencing stylesheets all over the place, it makes things unmanageable.

So with your example in mind, I'm now thinking of centralising everything into a single "master" .scss file, then reference that wherever it's needed. So JS files would only ever need to remember to use:

import styles from './assets/scss/styles.scss';

Then you can just reference the individual stylesheets from within the master .scss file, ie:

@import "vars";
@import "mixins";
@import "miniplayer";

I'm assuming this should just work as it's standard sass. Will also mean the files are a bit better organised

Any issues with that you can think of?

Again, this only applies to our core styles. Theming is another discussion for another day 😛

@jacobwgillespie
Copy link
Member

So, it's actually intentional - welcome to web development June 2016 (tm) 😄 (the month being there because best practices change so quickly in this world)

The idea with React components (and web components in general) is that each component is its own self-contained unit that encapsulates behavior and presentation into one specific package. So CSS (or whatever display properties) live right alongside the JS and are used only for that specific component.

There are various benefits / reasons behind why you'd want to use CSS modules besides the fact that this project uses that architecture, but one of the biggest ones is that you no longer have a global namespace for your classes and as such can avoid any potential name collisions as each CSS file is "scoped" to its respective component. You don't have to do "object oriented CSS" / BEM / any other pseudo-scoping in your class names, you get true scoping. There are more reasons / better arguments for this kind of thing online - a quick Google search turned up https://css-tricks.com/css-modules-part-1-need/ as an intro article.

You'd actually cause yourself a major headache if you did import styles from './assets/scss/styles.scss'; because you'd eliminate all the benefits of scoping and cause each JS file to need to understand its relative position in the project - you'd have a global namespace again and would still need imports in each JS file.

Anyways, this CSS modules + react + webpack is quickly becoming a standard for this kind of architecture. Many people originally advocated for React inline styles, so JS would be responsible for presentation as a replacement for CSS, and while there is some theoretical arguments for why that is a good idea and not as crazy as it sounds, CSS modules provides a nice blend of using CSS directly (and extracting all styles to one CSS file in production) and scoping / modules / code organization.

When I first encountered this it just felt wrong, but as you get comfortable with locating styles next to the element (component) they style, it all makes sense and you'll never want to have a global stylesheet again. :)

Also keep in mind that standard CSS cascading rules are preserved, so if you have structure like the following:

<App>
  <SongTitle />
</App>

and you apply a CSS property like font-family to the App, SongTitle will pick it up since it's a child element. Standard HTML / CSS behavior, so even though the stylesheets are separated, they operate as usual.

In general, the design pattern is you will have one "root" CSS file that styles generic elements (like this one) that aren't associated with specific classes / components, and then beyond that each component will have its own style file.

So yeah, it's kinda the architecture the project is using - we likely don't want to "globalize" the CSS file.

@chrismou
Copy link
Member Author

chrismou commented Jun 7, 2016

Ok cool, if that's what the cool kids are doing nowadays then I'll buy in! Still feels a bit wrong but I think it's probably because I'm thinking of it from a website point of view. When the files aren't traveling over the wire, transfer speed and caching probably isn't as much of an issue!

@chrismou chrismou closed this as completed Jun 7, 2016
@jacobwgillespie
Copy link
Member

Sweet - yeah, it definitely feels wrong at first until you get some experience with it. Keep in mind that in production, all those smaller SCSS files are processed and joined into one style.css file, so even in a website context, you still get a normal master CSS file with normal caching, etc., after using a module bundler like webpack (similar to how we're getting an app.js file from the combination of all the smaller JS files).

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

No branches or pull requests

2 participants