Skip to content

Incorrect path names being passed to CSS loader. #368

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

Closed
phyllisstein opened this issue Feb 8, 2017 · 7 comments
Closed

Incorrect path names being passed to CSS loader. #368

phyllisstein opened this issue Feb 8, 2017 · 7 comments

Comments

@phyllisstein
Copy link

After upgrading from 4.1.1 to 5.0.0, I noticed that the CSS loader, with modules enabled, was generating colliding hashes for different components in my application. I did a little poking around and discovered that the paths being passed to the loader are not accurate: when my localIdentName is set to [path][name]__[local]--[hash:base64], I see Webpack pulling in CSS from multiple files (e.g., app/components/logo-header/styles.scss and app/components/input/styles.scss, both of which define a class .root) but giving them the same identifier (e.g., .app-components-logo-header-styles__root--TqfiOWwF9tjvyCbNgxKoY, where one should have that identifier and the other should have .app-components-input-styles__root with a unique hash).

I'm working on putting together a minimum working example reproducing the bug, but I'd be grateful for any thoughts you had in the meantime. Thanks in advance for your help, and thanks for all your hard work on this repo!

@phyllisstein
Copy link
Author

This appears only to happen when using the data option. I've produced a working example at https://github.com/phyllisstein/sass-loader-collisions-bug.

@jhnns
Copy link
Member

jhnns commented Feb 8, 2017

Thx, I'll take a look.

@jhnns
Copy link
Member

jhnns commented Feb 8, 2017

Haha, nice one! Seems like you've encountered a bug/unexpected behavior of the loader-utils. Starting with commit e6dc9c1daa02b1c87d9431e77c7ae9771b22f80b, the loader utils will recognize loader options objects and just return them.

However, inside the sass-loader, I expected to always get a new object when calling parseQuery. That's why I modify the received options object. Thus, data will accumulate all sass files if the loader is executed multiple times.

@jhnns
Copy link
Member

jhnns commented Feb 8, 2017

This will bloat a lot of CSS files... working on a quickfix.

@jhnns
Copy link
Member

jhnns commented Feb 8, 2017

A fix should ship with 5.0.1

@jhnns jhnns closed this as completed Feb 8, 2017
@phyllisstein
Copy link
Author

Wow, that's super tricky! Thanks so much for the speedy response and fix---excited to upgrade.

@sontek
Copy link

sontek commented Mar 5, 2017

I'm getting this:

(node:98695) DeprecationWarning: loaderUtils.parseQuery() received a non-string value which can be problematic, see https://github.com/webpack/loader-utils/issues/56
parseQuery() will be replaced with getOptions() in the next major version of loader-utils.
    at Object.parseQuery (/Users/sontek/venvs/eventray/src/eventray/node_modules/loader-utils/index.js:78:3)
    at Object.module.exports (/Users/sontek/venvs/eventray/src/eventray/node_modules/babel-loader/lib/index.js:86:35)
    at LOADER_EXECUTION (/Users/sontek/venvs/eventray/src/eventray/node_modules/loader-runner/lib/LoaderRunner.js:119:14)
    at runSyncOrAsync (/Users/sontek/venvs/eventray/src/eventray/node_modules/loader-runner/lib/LoaderRunner.js:120:4)
    at iterateNormalLoaders (/Users/sontek/venvs/eventray/src/eventray/node_modules/loader-runner/lib/LoaderRunner.js:229:2)
    at iterateNormalLoaders (/Users/sontek/venvs/eventray/src/eventray/node_modules/loader-runner/lib/LoaderRunner.js:218:10)
    at /Users/sontek/venvs/eventray/src/eventray/node_modules/loader-runner/lib/LoaderRunner.js:233:3
    at Object.context.callback (/Users/sontek/venvs/eventray/src/eventray/node_modules/loader-runner/lib/LoaderRunner.js:111:13)
    at Object.module.exports (/Users/sontek/venvs/eventray/src/eventray/node_modules/eslint-loader/index.js:198:8)
    at LOADER_EXECUTION (/Users/sontek/venvs/eventray/src/eventray/node_modules/loader-runner/lib/LoaderRunner.js:119:14)
    at runSyncOrAsync (/Users/sontek/venvs/eventray/src/eventray/node_modules/loader-runner/lib/LoaderRunner.js:120:4)
    at iterateNormalLoaders (/Users/sontek/venvs/eventray/src/eventray/node_modules/loader-runner/lib/LoaderRunner.js:229:2)
    at Array.<anonymous> (/Users/sontek/venvs/eventray/src/eventray/node_modules/loader-runner/lib/LoaderRunner.js:202:4)
    at Storage.finished (/Users/sontek/venvs/eventray/src/eventray/node_modules/enhanced-resolve/lib/CachedInputFileSystem.js:38:15)
    at /Users/sontek/venvs/eventray/src/eventray/node_modules/enhanced-resolve/lib/CachedInputFileSystem.js:69:9
    at /Users/sontek/venvs/eventray/src/eventray/node_modules/graceful-fs/graceful-fs.js:78:16
    at FSReqWrap.readFileAfterClose [as oncomplete] (fs.js:416:3)

but my babel-loader config is this:

        {
            test: /\.jsx?$/,
            use: [
                {
                    loader: 'babel-loader',
                    options: {
                        presets: babelPresets,
                        plugins: babelPlugins
                    },
                }
            ],
            exclude: /node_modules/
        },

The two variables inside options look like this:

const getRules = (isDev, isTest) => {
    let babelPlugins = [];

    if (isTest === true) {
        babelPlugins = [
            ...babelPlugins,
            'istanbul'
        ];
    }

    if (isDev === true) {
        babelPlugins = [
            ...babelPlugins,
            'react-hot-loader/babel'
        ];
    }

    let babelPresets;

    /* In dev we don't need slow es2015 preset */
    if (isDev || isTest) {
        babelPresets = ['react', 'modern-browsers', 'stage-0'];
    }
    else {
        babelPresets = ['react', ['es2015', {"modules": false}], 'stage-0'];
    }

So I'm not seeing what is expected to be changed. My eslint-loader looks like this:

        {
            test: /\.jsx?$/,
            enforce: "pre",
            loader: 'eslint-loader',
            exclude: /node_modules/
        }

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

No branches or pull requests

3 participants