Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

includePaths: ['node_modules'] doesn't work for scoped packages #1596

Closed
XmlmXmlmX opened this issue Jun 17, 2016 · 18 comments
Closed

includePaths: ['node_modules'] doesn't work for scoped packages #1596

XmlmXmlmX opened this issue Jun 17, 2016 · 18 comments

Comments

@XmlmXmlmX
Copy link

XmlmXmlmX commented Jun 17, 2016

source/css/style.scss:

@import "@scope/package-name/scss/style.scss";

Gruntfile.js:

module.exports = function(grunt) {
    grunt.initConfig({
            sass: {
                options: {
                    sourceMap: true,
                    includePaths: ['node_modules']
                },
                dist: {
                    files: {
                        'source/css/temp/style.css': 'source/css/style.scss'
                    }
                }
            }
    });

    // Plugins
    require('load-grunt-tasks')(grunt);

    // Tasks
    grunt.registerTask('default', ['sass']);
};

Error when grunt:

Running "sass:dist" (sass) task
>> Error: File to import not found or unreadable: @scope/package-name/style.scss
>>        Parent style sheet: D:/source/css/style.scss
>>         on line 103 of source/css/style.scss
>> >> @import "@scope/package-name/scss/style.scss";
>>    ^

When using the same package without scope, it works fine.

@xzyfer
Copy link
Contributor

xzyfer commented Jun 17, 2016

What's the result of dir node_modules/@scope/package-name/scss?

@XmlmXmlmX
Copy link
Author

@xzyfer You mean what's inside the scss folder?

root\node_modules\@scope\package-name\scss>dir
20.06.2016  12:59    <DIR>          .
20.06.2016  12:59    <DIR>          ..
20.06.2016  12:59               679 mixins.scss
20.06.2016  12:59            13.009 style.scss
20.06.2016  12:59             4.414 variables.scss
               3 File(s)         18.102 bytes

@XmlmXmlmX
Copy link
Author

I got it. I've changed the includePaths value to node_modules/@scope. My includes are now relative to this path and it works.

@xzyfer
Copy link
Contributor

xzyfer commented Jun 21, 2016

Hmm I'm curious as to why didn't previously work. I'd like to rule out an issue with @ inside @import.

@anilanar
Copy link

anilanar commented Feb 6, 2017

Unfortunately scoped imports don't work with v4.5.0, it just cannot find the npm package.

@xzyfer
Copy link
Contributor

xzyfer commented Feb 6, 2017

@anilanar did it work before v4.5.0?

@anilanar
Copy link

anilanar commented Feb 6, 2017

@xzyfer Well right now, I'm trying to help our UI people to setup a lerna-based monorepo of stylesheets and I'm pretty confused.

Who owns the feature of tilde? Is it node-sass or is it webpack loaders/grunt plugins?

I assume node-sass has capability of custom importers and webpack loaders/grunt plugins implement custom importers.

So I was talking about tilde, so it's unrelated to node-sass right?

@xzyfer
Copy link
Contributor

xzyfer commented Feb 6, 2017

I have no idea what tilde is. If you want your import to be node_modules aware our official answer is to use eyeglass. Please take any future questions regarding eyeglass or loaders to stackoverflow or our slack (linked in to readme).

@XmlmXmlmX
Copy link
Author

@xzyfer tilde = ~.

@xzyfer
Copy link
Contributor

xzyfer commented Feb 6, 2017

Thanks @XmlmXmlmX.

@anilanar as @XmlmXmlmX points out adding node_modules/@scope to your include paths should address your issue in a quick and dirty way. I suggest using eyeglass for a more robust solution.

@alexander-akait
Copy link

alexander-akait commented Jun 6, 2017

@xzyfer That is, this problem will not be solved in node-sass? We have some problems in sass-loader for scoped packages. eyeglass seems abandoned 😞 I do not understand why we can't introduce this support, since it is the same as path, just contain one @character.

@yogeshgadge
Copy link

We need some sort of character e.g. ~ to represent the "library path" i.e. node_modules.

@Arikael
Copy link

Arikael commented Sep 7, 2017

you can try https://www.npmjs.com/package/node-sass-tilde-importer
like so (in gulpfile)

const tildeImporter = require('node-sass-tilde-importer');
function compileSass(path, ext, file, callback) {
    let compiledCss = sass.renderSync({
        file: path,
        importer: tildeImporter,
        outputStyle: 'compressed',
    });
    callback(null, compiledCss.css);
}

I just tested it and it seems to do the job (although I didn't test it with a scoped package)

@penx
Copy link

penx commented Apr 11, 2018

It would be good for the Sass language to officially infer that ~ means use your environment's package resolution, rather than this just being sass-loader specific syntax.

For node, adding a single node_modules to your path doesn't account for:

  • node_modules that may be in a parent folder (common in monorepo structures)
  • globally installed modules
  • when used with npm link

Note that this was requested in sass/sass#2350 but was closed with the comment:

As a rule, special resolution of imports are the domain of importer plugins, not the implementation itself. It's possible that this makes sense for a wrapper, such as node-sass, but not for the core language.

This seems to conflict with this issue having the tags Discussion - Proposal Invalid - Not in Sass

@xzyfer
Copy link
Contributor

xzyfer commented Apr 11, 2018 via email

@penx
Copy link

penx commented Apr 11, 2018

My concern is less about what build system I use, but more about distributing Sass libraries on npm that are dependent on other Sass libraries on npm, independent of what Sass build system the parent project uses to build them.

I guess eyeglass may be the solution, though introduces the assumption that the parent project uses node-sass and would use this. Perhaps you could argue that distributing Sass on npm infers that node-sass would be used for the build?

@penx
Copy link

penx commented Apr 11, 2018

~ is already an overloaded concept

sass-loader uses the concept that ~ without a slash is a lib import whereas ~/ is the home directory. That seems logical to me but I understand this introduces a breaking change.

@xzyfer
Copy link
Contributor

xzyfer commented Apr 11, 2018 via email

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

No branches or pull requests

8 participants