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

3.0.3 (PR #161) breaks gulp setup with bootstrap less file not found #166

Closed
dcousineau opened this issue Apr 24, 2015 · 8 comments
Closed

Comments

@dcousineau
Copy link

I've downgraded to 3.0.2 for now and if there's a preferred way to handle this now let me know


The 3.0.3 upgrade breaks my existing gulp system that previously worked with the error message

Error: "/node_modules/bootstrap/less/normalize.less" is not in the SourceMap.

The relevant snippet from my gulpfile:

gulp.task('less', function() {
    return gulp.src('application/static/src/less/core.less')
        .pipe(sourcemaps.init())
        .pipe(less({
            paths: ['node_modules', 'application/static/src/less']
        }).on('error', notify.onError({
            message: "Error: <%= error.message %>"
          , title: "LESS Compile Error"
        })))
        .pipe(gulpif(!production, sourcemaps.write('.')))
        .pipe(gulp.dest(destPath + '/css'))
        .pipe(buffer())
        .pipe(minifyCSS({
            keepSpecialComments: 0  //Drop everything including licensing comments, we will list in app
        }))
        .pipe(rename('core.min.css'))
        .pipe(gulp.dest(destPath + '/css'))
    ;
});

Which enabled us to do the following when we included bootstrap:

@import "bootstrap/less/variables.less";

If would love some direction on the preferred way to do this going forward. I don't see anything in the sourcemaps documentation to specify multiple paths and no way in PR #161 to turn off the new behavior.

@dcousineau dcousineau changed the title 3.0.3 (PR#161) breaks gulp setup with bootstrap less file not found 3.0.3 (PR #161) breaks gulp setup with bootstrap less file not found Apr 24, 2015
@yocontra
Copy link
Member

@Jenius

@jescalan
Copy link
Contributor

This is caused by #162, I think. Accord inlines all source content by default in order to prevent any issues with paths, like this one. When you make source map paths relative through less directly, accord will no longer inline sourcemaps, since this is an override of it's default source map handling and it can't guarantee the correct root to join along with the relative paths. It seems as if the relative path is not lining up correctly now with that change in place.

The way I see it, we have two options here. First is not to use relative paths and let accord handle source mapping its own way, using absolute paths, which works. However, it seems to me like there might be some benefit from having relative paths since a few people have requested it, although I'm not sure what it is. If someone could explain it to me, that would be great. A second potential option is to build a special case into the less adapter such that accord is aware of the relative path overrides and is able to correctly detect the root, join paths, and still inline the content. To be entirely transparent, this is not something I personally would be able to quickly complete, since it's a decent sized and I currently am traveling and don't have a ton of time free on my hands, especially for things that I don't entirely understand the benefit of.

Either way, I might roll back 3.0.3 if possible, for now. Changing the default for sourcemaps to relative paths is certainly very likely to be breaking for people either way, so it should not be a patch level update.

@dcousineau
Copy link
Author

I'll try some time to build out a minimal reproduction in the next few days so there's something to test against when this gets brought back in. Thanks!

@jrehwaldt
Copy link

How is the status on this one? Minifying a less file that was generated with 3.0.3 is impossible if sourcemaps should be preserved/generated. I tried with gulp-minify-css, gulp-cssnano and gulp-postcss+csswring... so it basically must be a gulp-less issue and as it works in 3.0.1 I believe it is related to this issue.

@yocontra
Copy link
Member

Can anyone confirm this is still a problem on the latest version?

jrehwaldt pushed a commit to jrehwaldt/gulp-minify-css-error that referenced this issue Oct 23, 2015
jrehwaldt pushed a commit to jrehwaldt/gulp-minify-css-error that referenced this issue Oct 23, 2015
@jrehwaldt
Copy link

Check out this repo where I initially prepared a testcase for gulp-minify-css, which I now extended to show regression caused by gulp-less@3.0.3 with either postcss or cssnano (both worked with 3.0.2). gulp-minify-css can be ignored as it was also broken pre-3.0.3.

@yocontra
Copy link
Member

Can you try with 3.0.4?

@jrehwaldt
Copy link

I tried my repo test case and the same effect persists. The minification fails silently and gulp never finishes the task.

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

5 participants