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

Passing includePath as an array #868

Open
StfBauer opened this issue May 25, 2023 · 1 comment
Open

Passing includePath as an array #868

StfBauer opened this issue May 25, 2023 · 1 comment

Comments

@StfBauer
Copy link

In my gulp task, I'd liked to do something like this.

        .pipe(
            $.if(!isProd, sass.sync({
                outputStyle: 'expanded',
                precision: 6,
                includePaths: module.paths,
                disableDeprecationWarnings: true
            }))
            .on('error', sass.logError))

module.paths is a predefined variable containing all the node_modules search paths to find installed packages. It returns already an array in the current form:

[
  '/Volumes/Code/customer/project/styleguide/node_modules',
  '/Volumes/Code/customer/project/node_modules',
  '/Volumes/Code/customer/node_modules',
  '/Volumes/Code/node_modules',
  '/Volumes/node_modules',
  '/node_modules'
]

This hasn't worked as expected, so I had to convert it to a string instead.

let incPath = module.paths.join(',');

Then I passed that into my sass compile task.

        .pipe(
            $.if(!isProd, sass.sync({
                outputStyle: 'expanded',
                precision: 6,
                includePaths: incPath,
                disableDeprecationWarnings: true
            }))
            .on('error', sass.logError))

I guess I found the issue here:

gulp-sass/index.js

Lines 122 to 134 in c04bb67

// Ensure file's parent directory in the include path
if (opts.includePaths) {
if (typeof opts.includePaths === 'string') {
opts.includePaths = [opts.includePaths];
}
} else {
opts.includePaths = [];
}
opts.includePaths.unshift(path.dirname(file.path));
// Generate Source Maps if the source-map plugin is present

It only checks for strings rather than for a path array. Is it safe to submit a PR for this change, or is there any reason more technical reason behind the check for the string story?

Happy to contribute this small adjustment.

@chesio
Copy link

chesio commented Dec 3, 2024

It only checks for strings rather than for a path array.

There must be something else going wrong with your setup, because arrays are perfectly supported.

The code snippet above first checks if opts.includePaths property is set/non-empty. If the value is not set or is empty, the code makes sure that it is an (empty) array - see the else branch. If the value is non-empty, then there is another nested if check which checks whether the value is a string and in such case it is converted to array; else it is assumed the value is an array and nothing is done.

Btw. when you update to gulp-sass version 6, you will have to rename the option to loadPaths - see: https://github.com/dlmanning/gulp-sass/blob/master/index.js#L122-L131

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

2 participants