sighjs / sigh

multi-process expressive build system for the web and node.js, built using baconjs observables
209 stars 12 forks source link

`pipeline({ activate: true })` doesn't work inside of merge #33

Closed jmatsushita closed 8 years ago

jmatsushita commented 8 years ago

Hi there,

I'm not sure what's going on here but if I have the following sigh.js

// To use a plugin it must be declared as a global variable, some plugins are
// built-in and others are loaded by scanning package.json for entries
// beginning with "sigh-" or "gulp-".
var merge, glob, concat, write, env, pipeline, debug
var pipe

module.exports = function(pipelines) {

  pipelines['reject'] = [
    debug(),
    reject({ projectPath : /^.*\.git.*$/ }),
    reject({ projectPath : /^.*node_modules.*$/ })
  ]

  pipelines['build-works'] = [
    glob({ basePath: 'source' }, '**/*.*'), pipeline({ activate: true }, 'reject'), debug(),
    pipe('cat'),
    write('dest')
  ]

  pipelines['build-fails'] = [
    merge(
      [ glob({ basePath: 'source' }, '**/*.*'), pipeline({ activate: true }, 'reject'), debug() ]
    ),
    pipe('cat'),
    write('dest')
  ]

}

Then sigh build-works works but sigh build-fails fails silently without outputting anything. With verbose output it yields:

$ sigh build-fails -vvvv
 * running pipelines [ build-fails ] with 3 jobs
 * waiting for subprocesses to start
 * subprocesses started in 0.432 seconds
 * pipeline(s) complete: 0.447 seconds
insidewhy commented 8 years ago

Don't have time to look properly but that use of merge is definitely wrong.

insidewhy commented 8 years ago

Isn't

pipelines['build-fails'] = [
    merge(
      [ glob({ basePath: 'source' }, '**/*.*'), pipeline({ activate: true }, 'reject'), debug() ]
    ),
    pipe('cat'),
    write('dest')
  ]

The same as:

pipelines['build-fails'] = [
    glob({ basePath: 'source' }, '**/*.*'),
    pipeline({ activate: true }, 'reject'),
    debug(),
    pipe('cat'),
    write('dest')
  ]
insidewhy commented 8 years ago

Sorry I'm being totally stupid, I should read things properly ;)

insidewhy commented 8 years ago

The issue is here: https://github.com/sighjs/sigh/blob/master/src/api.js#L236

The code doesn't look for activations inside of merges.

insidewhy commented 8 years ago

Released 0.12.27 with the fix.