Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate async errors to webpack #78

Merged
merged 1 commit into from
Feb 24, 2017

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.

request = request.replace(/^-?!+/, "");
request = request.split("!");
var content = fs.readFileSync(request.pop(), "utf-8");
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
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