rollup/rollup-watch

Error catching

Closed this issue · 11 comments

zwhitchcox commented

Just curious as to why we are emitting errors as an event as opposed to returning a promise or logging them? Just spend about 2 hours trying to find an error without realizing it was going to emit an error event...

zwhitchcox commented

I would propose returning the rollup promise...I would also propose allowing people to pass in the rollup function, because most people import it like this:

import {rollup} from rollup

Maybe detect if rollup is a function, and if so use it as a function...I don't see anywhere in the code where we need the actual reference to the object, so, if so, that would be nice.

zwhitchcox commented

Also,

import rollup from 'rollup'

returns an error, so I don't even know how you're getting the rollup object

TrySound commented

@zwhitchcox To import rollup use

import * as rollup from 'rollup';

We won't improve api with such conditions. API should be clean and simple. But I think would be better to pass rollup function instead.

/cc @Rich-Harris

About promises. You can resolve it just once. But watcher is iterated again and again. So your propose is not the case.
Logging works when you use CLI. In this case it's the same as logging promise rejection.

zwhitchcox commented

So, we have to use the CLI or listen for the error even if we want to get the errors?

You're saying the promise is only going to catch the error once, but writing

.then(()=> {}, error => console.log(error))

is the same as writing

.then(()=>{})
.catch(err => console.log(err))

Correct?

Or am I missing something here?

TrySound commented

Yep, it's the same. And both will be called once. With event emitter you may call listeners any times.

TrySound commented

Different API for different purposes.

zwhitchcox commented

gotcha...modified my pr...how's that?

will detect if run by command line, and if not, will output error....

rollup already shows the error twice for me, don't want a third time XD

zwhitchcox commented

Yes, but it doesn't log the error withthe programmatic api, e.g.:

  1 // I roll up, I roll up, I roll up, Shawty I roll up                                                                                    
  2 import * as rollup from 'rollup'                                                                                                        
  3 import watch from 'rollup-watch'                                                                                                        
  4 import config from '../rollup.config.js'                                                                                                
  5 watch(rollup, config)                                                                                                                   
  6   .on('event', e => e.code === 'ERROR' && console.log(e))

unless you listen for the event...or is this the intended behavior...also, +1 for this not being the intended behavior haha

talmobi commented

In my small watcher wrapper I directly use rollups js API (without rollup-watch) and catch the errors from the rollup.rollup promise directly. Seems to work pretty OK since a rebuild is done on each change (using cache if available of course).

(and onwarns that pass if ( /Treating .+ as external dependency/.test( message ) ) return

2cents

zwhitchcox commented

Thank you!

Yeah, I actually did get rollup-watch to work, but now there's other problems with rollup that I can't figure out :|...So, I think I'm just gonna make my life a whole lot easier and use jspm, because I'm not developing a library XD