Skip to content

Conversation

@Pinpickle
Copy link
Contributor

@Pinpickle Pinpickle commented Feb 12, 2017

When compiling a module that has asynchronous errors (e.g. when an error results from using extend or include), the error will not go through Webpack, but will be thrown in some async handler somewhere.

This has two pretty big drawbacks:

  • When watching files for changes, the entire process will exit when an error is thrown, as opposed to continuing and waiting for the error to be fixed (this is a deal breaker)
  • The error does not go through Webpack's error reporting, which some tools may leverage.

I've fixed this by making all thrown errors in the run function go through the Webpack callback. I've added tests to check that this behaviour works, and I have verified that it fixed the problem in a project I am using pug-loader in.

I had to modify the mock Webpack loader system to actually be asynchronous, all tests should still be passing.

Edit: Turns out this fixes #77 as well

@Pinpickle
Copy link
Contributor Author

Looks like the Travis errors are because there is no .travis.yml?

Copy link
Member

@TimothyGu TimothyGu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, other than a nit.

if(request[0] && /stringify/.test(request[0]))
content = JSON.stringify(content);
return callback(null, content);
process.nextTick(() => loadCallback(null, content));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While at it, you can use fs.readFile() instead of fs.readFileSync() too.

@TimothyGu
Copy link
Member

Yeah, don't worry about Travis for now. I'll fix it when I find the time too. Thanks a lot!

@Pinpickle Pinpickle force-pushed the propagate-async-error branch from 1834f3d to bf005ca Compare February 13, 2017 12:15
@Pinpickle
Copy link
Contributor Author

@TimothyGu I've made the file reading async now, as well as making resolve async in the mock runtime. How does this look now?

@TimothyGu TimothyGu merged commit 7459460 into pugjs:master Feb 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

webpack-dev-server fails on pug syntax error

2 participants