rollup / rollup-watch

Fast incremental rebuilds with Rollup CLI
MIT License
91 stars 23 forks source link

rollup-watch writing logs to stderr instead of stdout #11

Open talmobi opened 8 years ago

talmobi commented 8 years ago

Hello!

TL;DR: Could rollup-watch use stdout when logging basic info on watch/bundles and only stderr when an actual build error occurs?

I'm running "rollup -w -c" with a child_process.spawn and listening to its stdout and stderr streams.

the stdout stream is never used and only stderr is used. This is confusing when detecting errors.

For example on successful bundling a two lines are logged

bundling...
bundled in 37ms. Watching for changes...

But these messages are all piped through stderr (and not stdout as one would expect).

When an actual error occurs the stderr is used again (as expected) -- but in addition the streams are also closed. So the only way to detect an error has occurred is to listen for the spawns 'end' event and then re-execute "rollup -w -c" manually once a change has occurred (since rollup -w -c currently does not itself recover from errors (but that's not the point here)).

For example, stylus works as expected in that it uses stdout to log basic info when it is watching and bundling, and when an error occurs it uses stderr.

stylus -w -r -i main.styl -o bundle.css

I looked at the source and couldn't find what I was looking for -- could this be an issue with rollup itself as opposed to rollup-watch?

Thanks a bunch!

talmobi commented 8 years ago

To work around this issue I'm using this temporary fix in my watcher:

    var buffer = ''
    child.stderr.on('data', function (chunk) {
      console.log('child.stderr: ' + chunk.toString('utf8'))
      buffer += chunk.toString('utf8')

      // check if actual error or no
      // (rollup currently only uses stderr for everything, even non-errors)
      if (buffer.toLowerCase().indexOf('error') >= 0) {
        // was error
       child.__trigger(buffer) // live reload client with build error message
      } else {
        // wasn't error
        buffer = ''
        child.__trigger() // live reloads client with updates
      }
    })

    // this is what stdout looks like
    child.stdout.on('data', function (chunk) {
      console.log('child.stdout: ' + chunk.toString('utf8'))
      child.__trigger() // live reloads client with updates
    })

Is a bit hacky but works fine.

talmobi commented 8 years ago

test case of issue:

var cp = require('child_process')
var cmd = 'rollup -w -i index.js -o bundle.js'

cp.spawn('touch', ['index.js']) // create test source file..

cmd = cmd.split(' ')
var spawn = cp.spawn(cmd[0], cmd.slice(1))

// listen on stdout (these are never used by rollup-watch)
spawn.stdout.on('data', function (chunk) {
  var s = chunk.toString('utf8')
  console.log('spawn stdout: ' + s)
})
spawn.stdout.on('end', function () {
  console.log('spawn stdout: ' + '---END---')
})

// listen on stderr
spawn.stderr.on('data', function (chunk) {
  var s = chunk.toString('utf8')
  console.log('spawn stderr: ' + s)
})
spawn.stderr.on('end', function () {
  console.log('spawn stderr: ' + '---END---')
})

spawn.on('close', function (code) {
  console.log('SPAWN CLOSED')
})
Victorystick commented 8 years ago

The reason that Rollup logs messages to stderr is so that it can write the bundles straight to stdout; which is useful for piping. Though I suppose that piping isn't as useful for incremental builds.

jaap3 commented 8 years ago

Yeah, this should probably be closed. Writing diagnostics, logging and errors to stderr is absolutely the right thing to do.

talmobi commented 8 years ago

I don't know. Stylus is able to handle both cases. At the very least logging messages should perhaps be prefixed with "log: " so that they could more easily be ignored as not being actual errors.

talmobi commented 8 years ago

Specifically when the watch flag -w is used I don't think the piping mechanism makes much sense? I created a small wrapper module called wrollup (https://www.npmjs.com/package/wrollup) with similar logging to that of the stylus-lang watcher -- quite simple and seems to work well for me (requires a rollup.config.js)