shakacode / bootstrap-sass-loader

Webpack Loader for the Sass version Twitter Bootstrap
MIT License
118 stars 25 forks source link

$brand-* doesn't get overwritten #9

Closed mmahalwy closed 9 years ago

mmahalwy commented 9 years ago

I don't know why, but $brand-primary has no affect.

Setup:

// Example file. Copy this to your project
module.exports = {
  verbose: true, // Set to true to show diagnostic information

  // IMPORTANT: Set next two configuration so you can customize
  // bootstrapCustomizations: gets loaded before bootstrap so you can configure the variables used by bootstrap
  // mainSass: gets loaded after bootstrap, so you can override a bootstrap style.
  // NOTE, these are optional.

   bootstrapCustomizations: "src/styles/bootstrap.scss",
   mainSass: "src/styles/main.scss",

  // Default for the style loading
  styleLoader: "style-loader!css-loader!sass-loader",
  //
  // If you want to use the ExtractTextPlugin
  //   and you want compressed
  //     styleLoader: ExtractTextPlugin.extract("style-loader", "css-loader!sass-loader"),
  //
  // If you want expanded CSS
  //   styleLoader: ExtractTextPlugin.extract("style-loader", "css-loader!sass?outputStyle=expanded"),

  scripts: {
    'transition': true,
    'alert': true,
    'button': true,
    'carousel': true,
    'collapse': true,
    'dropdown': true,
    'modal': true,
    'tooltip': true,
    'popover': true,
    'scrollspy': true,
    'tab': true,
    'affix': true
  },
  styles: {
    "mixins": true,

    "normalize": true,
    "print": true,

    "scaffolding": true,
    "type": true,
    "code": true,
    "grid": true,
    "tables": true,
    "forms": true,
    "buttons": true,

    "component-animations": true,
    "glyphicons": true,
    "dropdowns": true,
    "button-groups": true,
    "input-groups": true,
    "navs": true,
    "navbar": true,
    "breadcrumbs": true,
    "pagination": true,
    "pager": true,
    "labels": true,
    "badges": true,
    "jumbotron": true,
    "thumbnails": true,
    "alerts": true,
    "progress-bars": true,
    "media": true,
    "list-group": true,
    "panels": true,
    "wells": true,
    "close": true,

    "modals": true,
    "tooltip": true,
    "popovers": true,
    "carousel": true,

    "utilities": true,
    "responsive-utilities": true
  }
};
/*
 * Webpack development server configuration
 *
 * This file is set up for serving the webpack-dev-server, which will watch for changes and recompile as required if
 * the subfolder /webpack-dev-server/ is visited. Visiting the root will not automatically reload.
 */
'use strict';
var webpack = require('webpack'),
    path = require('path'),
    ExtractTextPlugin = require("extract-text-webpack-plugin");

module.exports = {

  output: {
    filename: 'app.js',
    publicPath: '/assets/'
  },

  cache: true,
  debug: true,
  devtool: false,
  entry: [
      'webpack/hot/only-dev-server',
      './src/scripts/app.js',
      'bootstrap-sass!./bootstrap-sass.config.js'
  ],

  stats: {
    colors: true,
    reasons: true
  },

  resolve: {
    extensions: ['', '.js'],
    alias: {
      'styles': __dirname + '/src/styles',
      'components': __dirname + '/src/scripts/components',
      'actions': __dirname + '/src/scripts/actions',
      'stores': __dirname + '/src/scripts/stores',
      'constants': __dirname + '/src/scripts/constants'
    }
  },
  module: {
    preLoaders: [{
      test: /\.js$/,
      exclude: /node_modules|bootstrap-sass.config.js/,
      loader: 'jsxhint'
    }],
    loaders: [{
      test: /\.js$/,
      exclude: /node_modules|bootstrap-sass.config.js/,
      loader: 'react-hot!jsx-loader?harmony!babel-loader?optional=runtime'
    }, {
      test: /\.scss/,
      loader: 'style-loader!css-loader!sass-loader?outputStyle=expanded'
    }, {
      test: /\.css$/,
      loader: 'style-loader!css-loader'
    }, {
      test: /\.(png|woff|woff2|eot|ttf|svg|jpg)$/,
      loader: 'url-loader?limit=8192'
    },
    { test: /bootstrap\/js\//, loader: 'imports?jQuery=jquery' },
    // Needed for the css-loader when [bootstrap-webpack](https://github.com/bline/bootstrap-webpack)
    // loads bootstrap's css.
    { test: /\.woff(\?v=\d+\.\d+\.\d+)?$/,   loader: "url?limit=10000&minetype=application/font-woff" },
    { test: /\.ttf(\?v=\d+\.\d+\.\d+)?$/,    loader: "url?limit=10000&minetype=application/octet-stream" },
    { test: /\.eot(\?v=\d+\.\d+\.\d+)?$/,    loader: "file" },
    { test: /\.svg(\?v=\d+\.\d+\.\d+)?$/,    loader: "url?limit=10000&minetype=image/svg+xml" }
    ]
  },

  plugins: [
    new webpack.HotModuleReplacementPlugin(),
    new webpack.NoErrorsPlugin(),
    new webpack.ProvidePlugin({
        $: "jquery",
        jQuery: "jquery",
        "windows.jQuery": "jquery"
    }),
    new ExtractTextPlugin("main.css")
  ]

};
mmahalwy commented 9 years ago

I think in styles.loader.js, the custom file needs to come before the variables file Notice: https://github.com/twbs/bootstrap-sass/blob/master/assets/stylesheets/bootstrap/_variables.scss#L18

If it comes before, it overrides it. Otherwise, for some reason, it's not getting pushed down?

justin808 commented 9 years ago

@mmahalwy Order is important for sass. That's why there's two separate customization files. One for variables, and one for overriding the styles after they are declared. However, I think your point is valid.

The issue is that $brand-primary is being used by the other styles. I've only set $brand-primary and the other styles explicitly.

Based on checking the docs, I'd agree that we should change the order.

If I create a branch, will you try it out? Alternately, can you create a branch and create a PR? It would be nice to modify the example as well to show this use case.

Here's a reference: https://robots.thoughtbot.com/sass-default

justin808 commented 9 years ago

Actually, I'll create a branch. Very simple.

justin808 commented 9 years ago

@mmahalwy I'm not sure what to do about this one. I just tried, but in my use cases, I always refer to other bootstrap defined variables in my customization file. Check this out:

https://github.com/justin808/bootstrap-sass-loader-example/blob/master/_bootstrap-customizations.scss#L6

I'm guessing that I should just add another optional customization file. How's this look:

  // Use preBootstrapCustomizations to change $brand-primary. Ensure this preBootstrapCustomizations does not
  // depend on other bootstrap variables.
  preBootstrapCustomizations: "./_pre-bootstrap-customizations.scss",

  // Use bootstrapCustomizations to utilize other sass variables defined in preBootstrapCustomizations or the 
  // _variables.scss file. This is useful to set one customization value based on another value.
  bootstrapCustomizations: "./_bootstrap-customizations.scss",
justin808 commented 9 years ago

See https://github.com/justin808/bootstrap-sass-loader/pull/10

@mmahalwy Please try this and if it works for you, I'll push a new build.

See http://forum.railsonmaui.com/t/debugging-nodejs-and-webpack-loaders/142 for how to npm link to setup local copies, or specify the repo and branch in your package.json file.

I also created this to demonstrate the fix: https://github.com/justin808/bootstrap-sass-loader-example/pull/3

Thanks for the input!

justin808 commented 9 years ago

@mmahalwy If you look at this example, you can see how the package.json references the branch: https://github.com/justin808/bootstrap-sass-loader-example/pull/3

bootstrap-sass-loader": "justin808/bootstrap-sass-loader#fix-order-defaults-sass-import",

2__default__git_show__git_

mmahalwy commented 9 years ago

@justin808 going to give it a try later this evening. Sorry, lazt sunday and I am out :)

I'll try the PR and the examples and +1?

mmahalwy commented 9 years ago

Solved!