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

Less compilation doesn't break on missing variable #118

Closed
mike-engel opened this issue Dec 17, 2014 · 27 comments
Closed

Less compilation doesn't break on missing variable #118

mike-engel opened this issue Dec 17, 2014 · 27 comments
Labels

Comments

@mike-engel
Copy link

Not sure if this issue should be here or in the less repo, but here it is. I recently did a lot of refactoring and after deleting a certain file, the less task would never finish, which would cause more problems in the default task as you can imagine. After going through all the changes, I found that I had deleted a variable that was used in another file. Even though less should have thrown an error about a missing variable, it didn't.

I'm not sure if I've missed something with less 2.0.0 or if this is a bug between one or both repos.

Thanks!

@stephenlacy
Copy link
Contributor

Please run the files through the less CLI.
This plugin is just a through stream to less, all errors produced by less are emitted by this plugin.

@yocontra
Copy link
Member

^ Check to see if this reproduces on the CLI. If so file the issue on the less repo, if not it is a problem with the plugin and we will handle it here

@mike-engel
Copy link
Author

Indeed it looks like a gulp-less issue, as using lessc did throw an error. Here's the output:

% lessc less/hub.less ~/Desktop/hub.less                                   HEAD » 10:41:41
NameError: variable @conversations is undefined in /Users/Mike/less/modules/conversations.less on line 248, column 23:
247     font-weight: 600;
248     background-color: @conversations;
249     color:@white;

@yocontra
Copy link
Member

So an undefined variable is not printing an error to the console.

Can you post the logs from when this happens?

@mike-engel
Copy link
Author

Here's my gulp log. Notice how CSS never finishes and Default never starts. I've edited it down since tests print to the console and yada yada yada.

Mike:hub/1.4
% gulp                                                                                     master » 11:16:47
[10:38:15] Using gulpfile ~/Projects/uxe-vagrant/prototypes-site/gulpfile.js
[10:38:15] Starting 'css'...
[10:38:15] Starting 'templates'...
[10:38:15] Finished 'templates' after 3.12 ms
[10:38:15] Starting 'images'...
[10:38:15] Finished 'images' after 1.33 ms
[10:38:15] Starting 'scripts'...
subString:  [ 'console.log', '$log' ]
subString:  [ 'console.log', '$log' ]
subString:  [ 'console.log', '$log' ]
[10:38:16] Starting 'test'...
[10:38:16] Starting 'browser-sync'...
[10:38:16] Finished 'browser-sync' after 36 ms
[10:38:16] gulp-notify: [Gulp notification] Webpack gulped successfully
[BS] Proxying: http://localhost:1338
[BS] Now you can access your site through the following addresses:
[BS] Local URL: http://localhost:1403
[BS] External URL: http://192.168.131.88:1403
[BS] Watching files...
[10:38:18] gulp-notify: [Gulp notification] Webpack gulped successfully
[BS] File changed: lib/js/miscScripts.js
[10:38:19] Version: webpack 1.4.13
     Asset   Size  Chunks             Chunk Names
helpers.js  16871       0  [emitted]  main
[BS] File changed: lib/js/helpers.js
[10:38:19] gulp-notify: [Gulp notification] Images gulped successfully
[10:38:19] Version: webpack 1.4.13
    Asset    Size  Chunks             Chunk Names
ui.js  101724       0  [emitted]  main
[10:38:20] Version: webpack 1.4.13
           Asset    Size  Chunks             Chunk Names charts.js  394765       0  [emitted]  main
[BS] File changed: lib/js/ui.js
[10:38:20] gulp-notify: [Gulp notification] Images gulped successfully
[10:38:20] gulp-notify: [Gulp notification] Webpack gulped successfully
[BS] File changed: lib/js/charts.js
[10:38:21] Finished 'scripts' after 5.46 s
[10:38:21] gulp-notify: [Gulp notification] Images gulped successfully
[10:38:21] Finished 'test' after 5.62 s
[10:38:21] gulp-notify: [Gulp notification] Images gulped successfully
[10:38:22] gulp-notify: [Gulp notification] Images gulped successfully
[10:38:22] gulp-notify: [Gulp notification] Images gulped successfully
[10:38:23] gulp-notify: [Gulp notification] Images gulped successfully
[10:38:23] gulp-notify: [Gulp notification] Images gulped successfully
^[^C⏎

And here's my gulpfile for css and default tasks:

gulp.task('default', [ 'css', 'templates', 'images', 'scripts', 'test', 'browser-sync' ], function () {
    'use strict';

    process.stdout.write( 'The app has been gulped successfully\n' );

    gulp.watch(src.lib + 'less/**/*.less', ['css']);
    gulp.watch(src.lib + 'templates/**/**/*.ejs', ['templates']);
    gulp.watch(src.lib + 'js/**/*.js', ['scripts','test']);
    gulp.watch(src.lib + 'img/**/*', ['images']);
});
gulp.task('css', function () {
        return gulp.src(src.lib + 'less/' + src.project + '.less')
            .pipe(less({
                sourceMap: true,
                ie8compat: true
            }))
            .pipe( csslint({
                'box-model': false,
                'universal-selector': false,
                'regex-selectors': false,
                'unqualified-attributes': false,
                'box-sizing': false,
                'unique-headings': false,
                'qualified-headings': false,
                'font-sizes': false,
                'important': false,
                'overqualified-elements': false,
                'duplicate-properties': false,
                'duplicate-background-images': false,
                'outline-none': false,
                'adjoining-classes': false,
                'floats': false,
                'text-indent': false,
                'known-properties': false, // see https://github.com/stubbornella/csslint/issues/283 for why we had to add this, it's stupid
                'gradients': false, // our less mixin doesn't support the old webkit gradient syntax, so we're silencing this for now
                'font-faces': false // This has a max allowance of 10 font face declarations, which doesn't make sense for proper font families. Silencing
            }))
            .pipe( csslint.reporter() )
            .pipe(cssmin({
                showLog: true
            }))
            .pipe(gulp.dest(cwd + 'lib/css/'))
            .pipe(notify({
                message: 'CSS gulped successfully'
            }));
    });

@yocontra
Copy link
Member

Can you get me just the output of running gulp css? I don't need the output of all of the other tasks

@yocontra
Copy link
Member

Also you should use gulp-sourcemaps, sourceMap: true to less won't really work the way you think

@mike-engel
Copy link
Author

I understand that sourcemap was deprecated in v2, just haven't had time to switch it out :)

As for the log, It's a little hard to provide a good example for you, since my team is dependent on the --cwd flag. It actually seems to finish by itself, but also doesn't seem to write to a file, and I don't know why.

% gulpnocwd css                                                                            master » 13:32:54
[13:33:25] Working directory changed to ~/Projects/uxe-vagrant/prototypes-site
[13:33:28] Using gulpfile ~/Projects/uxe-vagrant/prototypes-site/gulpfile.js
[13:33:28] Starting 'css'...
[13:33:28] Finished 'css' after 17 ms

@yocontra
Copy link
Member

@mike-engel Is that with the error still in the less code?

@mike-engel
Copy link
Author

Yeah. If I run the default task, the css task never finishes. If I run it with the CLI, it errors out.

@yocontra yocontra added the bug label Dec 19, 2014
@umefarooq
Copy link

i have the same problem i have define variables it always go in my style task never comes out without defining variables its work fine. its not giving error on any undefined variable.

@mike-engel
Copy link
Author

Update: This also seems to be happening with @imports that can't find the source file.

@mike-engel
Copy link
Author

Doing some debugging tonight, and it seems as though this may be a bug with one of gulp-less' dependencies and not gulp-less itself. If I replace the return with a process.exit( 1 ); on line 72, the process does exit normally. This would lead me to believe it's a problem with through2 or the way it's implemented in gulp-less.

@kahwee
Copy link
Contributor

kahwee commented Dec 30, 2014

I added the following in gulpfile.js as an interim fix

        gulp.src('app.less')
            .pipe(less({
                compress: true
            }))
            .on('error', function(err) {
                console.log(chalk.red('Error with LESS.js'), err.message);
            })
            .pipe(gulp.dest('.'));

This prints an error through console.log. Something probably regressed in LESS.js.

Alternatively, tying it to LESS to ^1.3.7 also works.

@yocontra
Copy link
Member

Might be a problem with https://github.com/plus3network/gulp-less/blob/master/index.js#L72

Using the callback pattern has some weird behavior nobody has been able to track down. Can you try replacing that with a this.emit('error', whatever') and see if it corrects it

@mike-engel
Copy link
Author

@contra no good. Only process.exit() will completely stop the gulp process so far.

@yocontra
Copy link
Member

@mike-engel Wait you don't have a watch open or something right? Tasks are supposed to be able to fail and not crash the process. If a task failed and the process died while you had other shit going on that would be a bug. Not crashing is expected behavior unless there is nothing else being processed

@yocontra
Copy link
Member

I think you are conflating different things here

  • the plugin fails with an error, but the error is not making its way to the task system to be logged
  • the plugin failing does not exit the process

Lets focus on number 1 and not go off into the weeds. #2 is expected behavior.

@mike-engel
Copy link
Author

Well @contra, gulp-less < 2.0.0 did both on an error. It would report the error as well as kill the process. I'm fine with either of those scenarios, but changing the cb to the emit didn't report the error either.

@lorenmh
Copy link

lorenmh commented Jan 5, 2015

I'm using v2.0.1, and also experienced 'issue number 1'; I had a simple error when I was testing things out, and gulp-less didn't output anything.

gulp-less v: 2.0.1
gulp v: 3.8.10

The gulp task (called by using 'gulp concat-css' from the command line):

gulp.task('concat-css', function() {
  gulp.src( './css/pg/test.less' )
    .pipe( less() )
    .pipe( gulp.dest('./test') );
});

Gulp's output:

[12:24:28] Using gulpfile ~/some/path/gulpfile.js
[12:24:28] Starting 'concat-css'...
[12:24:28] Finished 'concat-css' after 8.54 ms

The .less file, test.less:

/* Note: the error was with this import, because this file actually didn't exist */
@import "../test-mod.less";

.blah {
  background-color: green;
}

The error output for lessc:

FileError: '../test-mod.less' wasn't found in /some/path/from/root/css/pg/test.less on line 1, column 1:
1 @import "../test-mod.less";
2

@magicdawn
Copy link

I don't understand what's doing in the index.js

cb(error)

since we can't see the error. why not just use gutil.log tell us what's wrong...

@magicdawn
Copy link

here is the problem : plugin catch error & pass to the callback & no error show on console

@yocontra
Copy link
Member

I have a test case that is reproducing this but I have no idea why the error isn't ending the stream - looking into it, no need to keep bumping this thread and sending everyone notifications

@yocontra
Copy link
Member

fixed in 2.0.3

@austinpray
Copy link

@retlehs

@mike-engel
Copy link
Author

Many thanks @contra and @ohJeez.

@umefarooq
Copy link

thanks for fixing

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

Successfully merging a pull request may close this issue.

8 participants