Eyeglass shares variable values between multiple entries when used with webpack

[dandel10n]

Package
This issue is related to the following monorepo package(s):

• eyeglass
• broccoli-eyeglass
• ember-cli-eyeglass

Description
I have two sass entries in webpack config, one with default theme styles and another one with $theme variable overwrite.

Webpack-chain scss config looks like this:

const MiniCssExtractPlugin = require('mini-css-extract-plugin');
const FixStyleOnlyEntriesPlugin = require('webpack-fix-style-only-entries');
const eyeglass = require('eyeglass');
const assets = require('postcss-assets');

const scssEntries = {
    entry: {
        'theme.one': './Assets/scss/theme.one.scss',
        'theme.two': './Assets/scss/theme.two.scss'
    }
};

const configureStylesLoader = config => {
    config
        .merge(scssEntries);

    config.devtool('source-map');

    // lint rule
    config.module
        .rule('scssCompilation')
            .test(/\.scss$/)
            .use('extractCSS')
                .loader(MiniCssExtractPlugin.loader)
                .end()
            .use('css')
                .loader('css-loader?-url')
                    .options({
                        sourceMap: true
                    })
                .end()
            .use('postcss')
                .loader('postcss-loader')
                    .options({
                        plugins: () => [
                            assets()
                        ],
                        sourceMap: true
                    })
                .end()
            .use('sass')
                .loader('sass-loader')
                    .options(eyeglass({
                        includePaths: ['node_modules/'],
                        sourceMap: true
                    }))
                .end();

    config
        .plugin('extract')
            .use(MiniCssExtractPlugin, [{
                filename: 'css/[name].css'
            }])
            .end()
        // stops webpack producing an associated JS file for every CSS file being compiled
        .plugin('fixStyleOutputs')
            .use(FixStyleOnlyEntriesPlugin);

    return config;
};

module.exports = configureStylesLoader;

// theme.one.scss

$theme: 'one';

//import our scss file dependencies
@import 'test';

// theme.two.scss

$theme: 'two';

//import our scss file dependencies
@import 'test';

// _test.scss

body {
    background-color: red;
}

@if ($theme == 'two') {
    body {
        background-color: blue;
    }
}

After webpack build in dist folder I have two files (theme.one.css and theme.two.css) but both with the overwritten $theme variable. If I remove eyeglass from the config, files are built as expected (one with default styles and another one with $theme variable being overwritten). I would like to use eyeglass in some shared npm packages with sass.

Both compiled files with eyeglass in webpack config looks the same:

body {
  background-color: red; }

body {
  background-color: blue; }

Compiled files without eyeglass:

// theme.one.scss

body {
  background-color: red; }

// theme.two.scss

body {
  background-color: red; }

body {
  background-color: blue; }

Environment:

• OS: Unix
• Node version: 10.15.0
• Eyeglass version: 2.4.1

[dandel10n]

Here is a basic example repo for the issue: https://github.com/dandel10n/eyeglass-demo

[chriseppstein]

This was a weird issue and it took me a while to understand what was going on.

Eyeglass options aren't meant to be shared across files. It's supposed to be invoked per sass file that's being compiled. By invoking it once during the configuration stage of webpack, it's sharing a single instance of Eyeglass across all your Sass files. The bit that seems to confuse webpack is that the options that are returned is a class instance now instead of a simple pojo and the cloneDeep function doesn't seem to actually clone it per output file. This causes the contents of the Sass files to be accumulated in options.data (because of this line https://github.com/webpack-contrib/sass-loader/blob/master/lib/normalizeOptions.js#L37).

In this example, the contents that are rendered for the file theme.one.scss (which is rendered second) becomes:

$theme: 'two';

//import our scss file dependencies
@import 'test';
$theme: 'one';

//import our scss file dependencies
@import 'test';"

The variable gets set to a new value but because of eyeglass's "import once" feature, the second @import "test" does nothing.

Here's the work-around that got it to work better for me:

const path = require('path');
const eyeglass = require('eyeglass');

// migrates the instance properties to a pojo which doesn't seem to confuse `cloneDeep`
const sassOptions = Object.assign({}, eyeglass({
    includePaths: ['node_modules/']
}));

module.exports = {
    entry: [ './src/theme.two.scss', './src/theme.one.scss'],
    output: {
        path: path.resolve(__dirname, 'dist'),
        filename: 'bundle.js',
    },
    module: {
        rules: [
            {
                test: /\.scss$/,
                use: [
                    {
                        loader: 'file-loader',
                        options: {
                            name: '[name].css',
                        }
                    },
                    {
                        loader: 'extract-loader'
                    },
                    {
                        loader: 'css-loader?-url'
                    },
                    {
                        loader: 'sass-loader',
                        options: sassOptions
                    }
                ]
            }
        ]
    }
};

IMO the sass-loader should allow options to be a function that accepts the sass filename and returns options specific for that file. Then eyeglass() would get called per sass file like it's supposed to and this wouldn't be a problem. Also, I cannot guarantee there won't be other shared state in the eyeglass instance that causes other problems in your codebase now or in the future.

I contend this is a bug in the sass-loader implementation because it is not correctly making a copy of the object it was passed so it accumulates state.

The proposed feature at #231 is somewhat related because it allows files to share eyeglass setup.

[dandel10n]

@chriseppstein Thanks for looking into this!