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

Generate external stylesheets #1304

Merged
merged 1 commit into from
Jan 5, 2017
Merged

Conversation

colinskow
Copy link
Contributor

In production apps it is a bad practice to inline large CSS files to Javascript, as it can slow initial page load and cause unstyled HTML to flicker.

This PR adds the ability to put SASS or CSS files in the ./src/styles directory, import them into a module, and they will generate an external CSS file on production builds. On dev builds they are imported using the style-loader to support hot module reloading.

@shlomiassaf
Copy link
Contributor

shlomiassaf commented Dec 28, 2016

@colinskow I used a different take on this.

I used a convention to identify global SCSS files: some-style.global.scss

Then a loader:

  globalScss: {
    test: /\.global\.scss$/,
    exclude: /node_modules/,
    use: [
      'style-loader',
      'css-loader',
      postCssLoader(),
      'sass-loader',
      'sass-resources-loader'
    ]
  },

The component SCSS loader must also exclude: [/\.global\.scss$/],

Now all *.global.scss files are set in the head.

This is of course not a separate file for styles but it differentiates between global and component local styles.

Now, for having styles in a separate file, this is something I think the developer should decide.

I think we should put the extract plugin to target 1 file, all other files the dev wants to include he should import from that main (and not from JS code).

If he imports from JS code it goes to the head.

My reasoning about this is that if all styles are in 1 directory it's far more logical to import them all from one file inside that directory, similar to an indexfile in a package/module/barrel.

I see this directory as the theme for the app, plus it's more aligned with SASS @imports...

I have a problem with style imports scattered around the project, it's hard tracking their order and that's important for CSS.

@colinskow
Copy link
Contributor Author

colinskow commented Dec 29, 2016

The order of the CSS will be the order the files were imported. I've created a styles directory where all the external styles will originate, and I think we should leave it to the developer to choose a convention. They are not scattered around the project, except for the Angular component styles which are inlined with the JS.

I library like Bootstrap can be imported by putting a file in the styles directory and using @import.

@colinskow
Copy link
Contributor Author

@d3viant0ne it doesn't appear we have any disagreements with this PR. @shlomiassaf suggests using the naming convention *.global.scss for external stylesheets, but I believe this is not necessary with everything in the src/styles directory.

I suggest we merge this so I can start working on other enhancements like DLLs without having to rebase.

@joshwiens
Copy link
Contributor

1.) Rebase it.

2.) As far as conventions go, they are much like words in scrabble. Common Use

In this case, I think it would be best to not enforce conventions on people. If a developer wants to do something like @shlomiassaf is suggesting, they can extend the project as they see fit.

The most common way I have seen / used in regards to scss and 3rd party styles would be to use @import in something like main.scss / styles.scss at the root of a project or styles directory.

Keep it simple & let the developer make his / her own decisions on conventions.

@colinskow
Copy link
Contributor Author

Rebase completed.

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

Successfully merging this pull request may close these issues.

3 participants