Error in plugin appears to make watcher process exit under some conditions

[Flaise]

When using rollup-watch directly, without the CLI (which I am doing because I need events to fire in response to build events), errors raised in plugins appear to halt the watching process. Here's some code that should reproduce the issue:

// ./build.js
const rollup = require('rollup');
const watch = require('rollup-watch');
const buble = require('rollup-plugin-buble');

watch(rollup, {
    entry: './example/test.js',
    dest: './dist/output.js',
    plugins: [buble()]
}).on('event', event => console.log(event));

// ./example/test.js
export function a(b) {
    for (let c of b) {

    }
}

In terms of output, it displays the STARTING, BUILD_START, and ERROR messages like you might expect but then the Node process exits instead of waiting for me to go fix the input file.

Am I doing this right? It looks pretty much like the way Rollup's CLI implementation does it, except that the CLI does not end its process, for no reason that I can discern.

edit: Forgot to note that I've tested this on Node 5 and 6, Mac OS X, npm 3.8.6, rollup 0.32.0, rollup-watch 2.4.0, rollup-plugin-buble 0.11.0.

[Rich-Harris]

Sorry for delayed response. I think it's actually behaving the same as the CLI, in that it fails if the error is present when you first run build.js, but once it's had a successful run it will recover from errors gracefully.

The reason for that is that until it's built the bundle, it doesn't know the dependency graph, and so is unable to watch files. I haven't been able to think of a smart solution to that problem.

I don't know if that helps at all?

[tivac]

Could some sort of flag on the event be added to inform consumers that rollup-watch couldn't even finish the initial build?

Then at least I could shut down the dev server gracefully w/o having to track myself that the initial build went through successfully.

[sloanelybutsurely]

Coming here to 👍 this.

I've created a gist that you can download and reproduce this with the cli (before I realized this was already an open issue) https://gist.github.com/zperrault/573667b102e4b8fd20ebffd45030462f

@Rich-Harris Maybe I don't understand how this works well enough but it seems to me like even if the bundle has been built once it will still crash.

For example:

I'd be happy to take a closer look at this if you're looking for contributions.

[FND]

I've run into what seems like the same issue, though with syntax errors rather than anything plugin-related: → reduced test case

That behaves mostly like @Rich-Harris described above - except when using an editor:

Curiously, applying that same change in an editor [instead of using echo in-valid > index.js] also results in the Rollup process terminating.

Unfortunately, this makes rollup-watch a hard sell for day-to-day usage, as it induces anxiety about introducing syntax errors, even temporarily.

I've traced the source to Rollup's handleError, but process.exit is not being invoked there, so I'm stumped.

FWIW, I'd be okay with Rollup exiting immediately when the initial build fails if there's no simple way to fix that - it's the unexpected failure in the background that I'm worried about.

(Apologies if this turns out not to be the same issue, but I figured it might help to provide a little more data.)

[ralphholzmann]

yeah this is particularly frustrating. Going to see if I can PR.

[talmobi]

@FND opened an issue on wrollup linking to this issue and I did some investigations. https://github.com/talmobi/wrollup/issues/2

Basically everything works fine in rollup version 0.34.10 but in rollup version 0.36.1 there are some odd RangeError: Maximum callstack exceeded errors when caching is turned on and importing modules.

Test sample and reproduction steps here: https://github.com/talmobi/wrollup_issue_2

(wrollup is a small wrapper I've hacked together around rollup to make the watching/bundling process nicer (auto recover on initial builds etc) until rollup-watch is up to snuff.)

[talmobi]

v 0.34.13 seems to be working fine, but then it jumps to v0.35.0 that has the RangeError: Callstack exceeded issues.

One commit in between these releases strikes me as a potential cause for it with the commit message "clone ASTs, for fast incremental rebuilds": https://github.com/rollup/rollup/commit/83ccb9725374e0fde9d07043959c397b15d26c67

looking into it atm

[talmobi]

Yeah, ok, so I think I might have guessed right. Using `deepClone produces the errors.

If you look at the commit here: https://github.com/rollup/rollup/commit/83ccb9725374e0fde9d07043959c397b15d26c67#diff-5c98da346b849e07de8c1173579789b0L320

So reverting that line of code and not using deepClone (aka use the old line that is commented out in that commit) will fix the issue -- at least the RangeError: Maximum call stack exceeded errors will disappear and seems to work as expected.

Not exactly sure what good the deepClone is supposed to do here anyway -- or if reverting back to the old line of code is a bad thing? At least it seems to fix the issue.

Any insights on this?

[talmobi]

So basically this PR https://github.com/talmobi/rollup/commit/3eceff612fe96ddb2b99be5d6e8e12cc9bc1b9e9 should at least be a temporary fix.

Any insights?