rollup
rollup copied to clipboard
Using `this.error` outside the transformation context
We want to run some processes in buildStart
and watchChange
in a plugin that I work on. The process needs to emit diagnostics and wish to use the rollup API to show the user those diagnostics, but it always throws when we use this.error
outside the transform
hook.
Is there a way to use this API and recover from the error in other hooks?
No. What this.error
does is more or less just throw an Error, but add some additional attributes to the Error
instance.
But you can use this.warn
.
Out of curiosity: Is it possible to use this.error
in the transform
hook without failing the build? That should actually not be possible.
I guess there should be a distinction between failing the build and exiting the whole process with code 1.
For example, if the user is using watch mode you don't want to kill the process also if there was an error (To give the user the chance to fix the problem).
When you use this.error
inside the transform
hook it works as stated, but when used in watchChange
it kills the process.
Is there a way to mimic this behavior inside other hooks such as watchChange
and buildStart
?
It should not abort watchMode if you throw in buildStart
, but maybe it does? But still, why not use this.warn
? Or is the important part that you DO want to abort the build?
Ok, in my experiments, throwing in buildStart
does NOT abort watch mode and thus should behave exactly like transform
or load
etc.
It is only throwing in watchChange
that aborts watch mode. So is your issue actually that throwing in watchChange
should not abort watch mode but just display the error?
Created #4427 as a fix, please give it a go
Ok, in my experiments, throwing in
buildStart
does NOT abort watch mode and thus should behave exactly liketransform
orload
etc. It is only throwing inwatchChange
that aborts watch mode. So is your issue actually that throwing inwatchChange
should not abort watch mode but just display the error?
So regarding this, I did my investigation and you are right, the problem was that I was using this.error
before I used
this.addWatchFile
, when I switch between them it worked in the buildStart
. So thanks for that :)
Created #4427 as a fix, please give it a go
But as you stated, I still have a problem with the watchChange
, I tried to fork this repo and use the branch from this PR and it did not work on my example (Link to the repo), unfortunately.
I tried to fork this repo and use the branch
I was wondering why you did not simply run npm install rollup/rollup#gh-4424_throw_watch_change
as the bot suggest in the PR, but it turns out this was broken for npm 7, I fixed this now on the branch.
Otherwise, it kind of works for me (in the least, the watch process was never aborted), except for two things:
function onBuildStartPlugin() {
return {
name: 'onWatchThrow',
async buildStart() {
this.addWatchFile(filePath);
emitError();
},
async watchChange() {
emitError();
},
};
}
function emitError() {
const file = readFileSync(filePath, 'utf8');
if (file.includes("console.log('error')")) {
this.error(new Error('Stop the process'));
}
}
- Just calling
emitError()
will havethis
set to the wrong value. If you replace it above withemitError.call(this)
, the correct errors are thrown. -
watchChange
is a synchronous hook, therefore due to your use ofasync
I got your errors as unhandled Promise rejections. The reason is that the watcher uses events to communicate, i.e. thewatchChange
hook will be triggered via an event, and we cannot simply wait for an event listener Promise without massively rearchitecting stuff.
I tried to fork this repo and use the branch
I was wondering why you did not simply run
npm install rollup/rollup#gh-4424_throw_watch_change
as the bot suggest in the PR, but it turns out this was broken for npm 7, I fixed this now on the branch.Otherwise, it kind of works for me (in the least, the watch process was never aborted), except for two things:
function onBuildStartPlugin() { return { name: 'onWatchThrow', async buildStart() { this.addWatchFile(filePath); emitError(); }, async watchChange() { emitError(); }, }; } function emitError() { const file = readFileSync(filePath, 'utf8'); if (file.includes("console.log('error')")) { this.error(new Error('Stop the process')); } }
Just calling
emitError()
will havethis
set to the wrong value. If you replace it above withemitError.call(this)
, the correct errors are thrown.
watchChange
is a synchronous hook, therefore due to your use ofasync
I got your errors as unhandled Promise rejections. The reason is that the watcher uses events to communicate, i.e. thewatchChange
hook will be triggered via an event, and we cannot simply wait for an event listener Promise without massively rearchitecting stuff.
-
My bad :), tried to refactor the code to be cleaner and push it as fast as possible and it got messed up, but I see you got the point regardless.
-
I see, so this is pretty bad news for our case. We will still have to find a way to run the process which is asynchronous and still report the diagnostics for the user.
The preferred way is use the rollup api of course but I don't see how it's possible.
I will see how much work it would be to allow the watchChange
hook to become async.
I pushed a potential solution to #4427 which will make the watchChange
and closeWatcher
hooks async.
I pushed a potential solution to #4427 which will make the
watchChange
andcloseWatcher
hooks async.
Amazing, I tried this branch and it works as expected!