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

Problems with project-relative paths #149

Open
strarsis opened this issue Nov 14, 2016 · 5 comments
Open

Problems with project-relative paths #149

strarsis opened this issue Nov 14, 2016 · 5 comments
Labels
package:eyeglass Related to the core eyeglass package in this monorepo.

Comments

@strarsis
Copy link
Contributor

strarsis commented Nov 14, 2016

Using plain node-sass (gulp-sass), it is possible to specify project-relative paths:

Gulpfile.js:

var gulp = require("gulp");
var sass = require("gulp-sass");
var sassOptions = {};
gulp.task("sass", function() {
  gulp.src("./scss/**/*.scss")
      .pipe(sass(sassOptions).on("error", sass.logError))
      .pipe(gulp.dest("./css"));
});

./scss/styles.scss:

@import dir1/_file1.scss;

./scss/dir1/_file1.scss

@import dir2/_file2.scss;
.test1 {
  color: red;
}

./scss/dir2/_file2.scss:

.test2 {
  color: blue;
}

With ./scss being the topmost folder, import paths are correctly resolved.

$ gulp sass

./css/styles.css:

.file2 {
  color: blue; }

.file1 {
  color: red; }

When eyeglass is also used, these project-relative paths aren't relative to project-folder anymore:

var gulp = require("gulp");
var sass = require("gulp-sass");
var eyeglass = require("eyeglass");
var sassOptions = {
  // put node-sass options you need here.

  eyeglass: {
    // put eyeglass options you need here.
  }
};
gulp.task("eyeglass", function () {
  gulp.src("./scss/**/*.scss")
    .pipe(sass(eyeglass(sassOptions)).on("error", sass.logError))
    .pipe(gulp.dest("./css-eyeglass"));
});
$ gulp eyeglass

[...]
Error in plugin 'sass'
Message:
    scss/dir1/_file1.scss
Error: Error: Could not import dir2/_file2 from any of the following locations:
         /home/test/src/eyeglass-includepath-issue/scss/dir1/dir2/_file2.scss
         /home/test/src/eyeglass-includepath-issue/scss/dir1/dir2/_file2.sass
         /home/test/src/eyeglass-includepath-issue/scss/dir1/dir2/_file2.css
         /home/test/src/eyeglass-includepath-issue/scss/dir1/dir2/_file2/index.scss
         /home/test/src/eyeglass-includepath-issue/scss/dir1/dir2/_file2/index.sass
         /home/test/src/eyeglass-includepath-issue/scss/dir1/dir2/_file2/index.css
         /home/test/src/eyeglass-includepath-issue/scss/dir1/dir2/_file2/_index.scss
         /home/test/src/eyeglass-includepath-issue/scss/dir1/dir2/_file2/_index.sass
         /home/test/src/eyeglass-includepath-issue/scss/dir1/dir2/_file2/_index.css
        on line 1 of scss/dir1/_file1.scss
>> @import 'dir2/_file2';
   --------^

Sample project for reproducing this issue.

Edit: Using the eyeglass root option (e.g. root: __dirname or root: __dirname + '/scss') doesn't seem to change the way how these paths are interpreted.

@strarsis
Copy link
Contributor Author

strarsis commented Nov 16, 2016

I have to add the root dir/project folder as include path, too as workaround to get around this error.
See the updated sample project repository.

Also, root option must be either omitted for its default value or
the directory above the scss directory specified.

@eoneill
Copy link
Contributor

eoneill commented Nov 16, 2016

This is a "bug" in gulp-sass. See dlmanning/gulp-sass#474 for more context. gulp-sass (incorrectly) auto-magically adds the file's parent directory onto the includePaths. As this is not the intended behavior of Sass imports, we won't support this behavior in eyeglass import logic. You can work around this by manually adding the desired directory to the includePaths. gulp-sass has chosen to keep this behavior because it's a breaking change for their users.

@strarsis
Copy link
Contributor Author

strarsis commented Nov 16, 2016

Wouldn't it make sense to add the root path to includePaths so all files in root can be imported correctly?
Also a note in the eyeglass / gulp integration documentation would be helpful.

@eoneill
Copy link
Contributor

eoneill commented Nov 16, 2016

Wouldn't it make sense to add the root path to includePaths so all files in root can be imported correctly?

This isn't how Sass/node-sass behaves by default, so we do not want to make this the default behavior in eyeglass.

Here's an example using node-sass directly to compile your example project:

var sass = require("node-sass");
var path = require("path");

sass.render({
  file: path.join(__dirname, "scss", "styles.scss")
}, function(error, result) {
  if (error) {
    console.error(error);
  }
  else {
    console.log(result.css.toString());
  }
});

You'll notice that it throws a similar error: File to import not found or unreadable: dir2/_file2

Also a note in the eyeglass / gulp integration documentation would be helpful.

👍Happy to take a PR.

@strarsis
Copy link
Contributor Author

To what section in README this notice about adding the project folder to includePaths would belong?
And does it also make sense to add this to Gulpfile integration guide as the behaviour of gulp-sass changes when using it with eyeglass?

@chriseppstein chriseppstein added the package:eyeglass Related to the core eyeglass package in this monorepo. label Feb 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:eyeglass Related to the core eyeglass package in this monorepo.
Projects
None yet
Development

No branches or pull requests

3 participants