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

1.0.2 module resolve requires underbar for includePaths. #98

Closed
jsg2021 opened this issue May 18, 2015 · 10 comments
Closed

1.0.2 module resolve requires underbar for includePaths. #98

jsg2021 opened this issue May 18, 2015 · 10 comments

Comments

@jsg2021
Copy link

jsg2021 commented May 18, 2015

Just upgraded from 0.4, so I can get node-sass 3, but will need to drop back down.

I have bower install foundation to a vendor directory in my project. My app's stylesheet extends foundation.

./src/main/resources/scss/app.scss
./src/main/resources/vender/foundation/scss/foundation.scss

If i @import "foundation" ... (includePaths set) sass-loader looks for _foundation.scss instead of foundation.scss. (it should be using the name resolve pattern for each include path)

my grunt sass task completes just fine. (using the same node-sass lib 3.1.1)

@jhnns
Copy link
Member

jhnns commented May 20, 2015

Since sass-loader 1.0.0 you don't need to set the includePaths manually. It should be resolved like a webpack module.

The error about not finding _foundation.scss is misleading, I have to admit. But the sass-loader always tries to resolve the module without a preceding underscore first, then with underscore. So that's not the error (I probably have to rework the error messages).

Could you post your webpack.config.js?

@jsg2021
Copy link
Author

jsg2021 commented May 20, 2015

The things in the vendor directory are not things I expect WebPack to resolve? So for my case, I would expect I would need to set the includePaths.

I've pared it down to the essentials:

'use strict';

var webpack = require('webpack');
var path = require('path');

var scssIncludes =
    'includePaths[]=' + path.resolve(__dirname, './src/main/resources/vendor/foundation/scss');

var root = path.resolve(__dirname, './src/main/js');


var commonLoaders = [
    { test: /\.json$/, loader: 'json' },
    { test: /\.js(x?)$/,
        loader: 'babel?optional=runtime'
    },

    { test: /\.(ico|gif|png|jpg|svg)$/, loader: 'url?limit=100000&name=resources/images/[name].[ext]&mimeType=image/[ext]' },

    {
        test: /\.(eot|ttf|woff)$/,
        loader: 'file',
        query: {
            name: 'resources/fonts/[name].[ext]'
        }
    }

];



module.exports =
    {
        name: 'browser',
        output: {
            path: './stage/client/',
            filename: 'js/[hash].js',
            chunkFilename: 'js/[hash]-[id].js',
            publicPath: '/'
        },

        cache: true,
        debug: true,
        devtool: 'source-map',

        entry: './src/main/js/index.js',

        target: 'web',

        resolve: {
            root: root,
            extensions: ['', '.jsx', '.js', '.json', '.css', '.scss', '.html']
        },

        module: {
            loaders: commonLoaders.concat([
                { test: /\.css$/, loader: 'style!css' },
                { test: /\.scss$/, loader: 'style!css!sass?' + scssIncludes }
            ])
        }
    };

@jhnns
Copy link
Member

jhnns commented Jun 3, 2015

Mhmm it could be that includePaths is broken since 1.0.0 because webpack is responsible for resolving paths (I still need to confirm that in a test though). However, it should work if you do it the webpack-way: Add ./src/main/resources/vendor/ to resolve.root and then write @import "~foundation/scss/foundation"

@jsg2021
Copy link
Author

jsg2021 commented Jun 23, 2015

I would prefer to write the scss (and really, all my code) webpack-agnostically. Any updates?

@sogko
Copy link

sogko commented Jun 24, 2015

I'm facing the same issue, watching this thread.

@jhnns
Copy link
Member

jhnns commented Jun 30, 2015

I would prefer to write the scss (and really, all my code) webpack-agnostically

That's a valid argument...

I'll have a look as soon as I found some time. Unfortunately I'm loaded with work... 😒. In the meantime, you can also take a look for yourself. The code is not complicated, but it requires some understanding about how to write loaders for webpack.

@sogko
Copy link

sogko commented Jun 30, 2015

Hi guys,
@oopschen faced this same issue #110 and has provided a PR #113

The code seems sound but I don't have enough knowledge of the inner workings of the loader to say for sure that it would not have side effects.

@jsg2021
Copy link
Author

jsg2021 commented Jul 17, 2015

@jhnns Is #113 viable?

@jorrit
Copy link
Contributor

jorrit commented Jul 17, 2015

It looks like the maintainers are having a holiday. I hope they can review all PR's soon.

@jhnns
Copy link
Member

jhnns commented Jul 22, 2015

Should be fixed with 1.0.3

@jhnns jhnns closed this as completed Jul 22, 2015
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

4 participants