ck86 / gulp-bower-files

Build gulp.src() of your bower packages main files.
80 stars 13 forks source link

Missing positive glob #21

Closed Kristories closed 10 years ago

Kristories commented 10 years ago

Bower task

gulp.task('bower', function () {
    return bowerFiles()
        .pipe(gulp.dest(path.production + 'assets/vendor'));
});

Watching bower.json

gulp.watch('bower.json', ['bower']);

Uninstall all packages and get an error when dependencies is empty

Missing positive glob

ck86 commented 10 years ago

What exactly is the problem?

Kristories commented 10 years ago

Is there a way to handle errors when dependencies is empty?

ck86 commented 10 years ago

Why should I use this plugin when I have no dependencies? ;)

Kristories commented 10 years ago

:beers:

benhohner commented 10 years ago

I'll give a good reason: Because I'm writing a package where users can optionally specify bower dependencies. It would be understandable that the error testing logic would by my responsibility if the docs weren't worded this way:

This will read your bower.json, iterate through your dependencies and build a gulp.src() with all files defined in the main property of the packages bower.json

This implies that the gulp.src() will be empty if there are no dependencies, not that node will crash. The proper behaviour here would be to simply do nothing or emit a non-blocking warning. If two people were willing to report a bug, imagine how many other people may be affected?

Please give this some consideration. Thanks! :heart_eyes_cat:

srsgores commented 10 years ago

Hi, I have this same problem. How can I fix this??

torkristensen commented 10 years ago

This scenario should be handled, not just because because implementing it is an obvious test-case that the plugin fails to handle gracefully ("positive glob missing"? is that the best error message you can emit? \ really? **).

For our specific purpose this plugin is deployed as part of a gulp project that is scaffolded by slush.

Depending on selection of options and case, the resulting project MAY NOT HAVE bower components included initially. However, the gulp project should transparently merge bower files into the built application without requiring the developer to modify the build files.

Until this plugin can handle the case of no active dependencies and pass though $.util.noop() it's not really complete, and doesn't report its' failure in a clear way.

ck86 commented 10 years ago

This error is thrown in gulp (glob-stream), not in gulp-bower-files.

It would be the same if you try:

gulp.task('nonsense_task', function() {
    return gulp.src([]).pipe(gulp.dest('.'));
});
torkristensen commented 10 years ago

Generic CoffeeScript workaround, allows you to use the plugin in the cases where

buildBower:()->
    hasBower  =  fs.existsSync './bower_components'

    if not hasBower then console.log "YOU DO NOT HAVE ANY BOWER COMPONENTS IN USE"
    return true if not hasBower

    bowerconfig=JSON.parse fs.readFileSync './bower.json',encoding:'utf8'        
    return true if Object.keys(bowerconfig.dependencies).length==0

    doYourBowerStuffNow()
torkristensen commented 10 years ago

My issue is that the plugin is smart enough to find and parse the bower config, but just isn't clever enough to return something more informative than the upstream error. It doesn't take a lot more checking as my example shows.

ck86 commented 10 years ago

The question is: Why should I create a gulp task, which does something with bower packages, but I have no bower packages (yet)?

torkristensen commented 10 years ago

Any scenario where you are building a system (example: slush/yo generator) where the initial project may or may not have default bower packages based on user configuration.

The fragility of your plugin reduces its' usefulness.

ck86 commented 10 years ago

In 0.2.4 the plugin returns an empty stream, see: 5e670410669ae8d838a9041ce6513a0a4cb6b0dc

torkristensen commented 10 years ago

Looks promising, will test as soon as I get a moment! Thanks for update.