Adds BroccoliBuildError for better errors messages
#10
Conversation
|
@twokul this is a huge foundational piece that I had zero clue how to do. This is going to be fantastic. |
lib/builder.js
Outdated
| // it does not make sense to throw multiple build errors, | ||
| // we can just report a failure once. | ||
| if (!isThrown) { | ||
| isThrown = true; |
There was a problem hiding this comment.
Chatted with @twokul in slack, and he is going to update to ensure that this isThrown is reset when the next build starts.
| * broccoli plugins, such as error type, code frames, | ||
| * broccoli node name and package versions. | ||
| */ | ||
| function BroccoliBuildError(originalError, node) { |
There was a problem hiding this comment.
the whole repo doesn't use classes and still specifies older versions of node; I felt it would make sense to convert everything in one go rather than in one place.
lib/broccoli-build-error.js
Outdated
| * broccoli node name and package versions. | ||
| */ | ||
| function BroccoliBuildError(originalError, node) { | ||
| var packageInfo = require(path.join(process.cwd(), 'package.json')); |
There was a problem hiding this comment.
process.cwd() can be literally anywhere not just in the projects root. I don't believe this approach will work.
I also don't feel broccoli should have any awareness of ember-cli, so a more loosely coupled solution is required.
I suspect rather, we may need to provide an option to broccoli-builder/middleware, so that ember-cli can provide a callback to "add additional information" to errors on the way out.
lib/broccoli-build-error.js
Outdated
| emberCLIVersion: null, | ||
| emberVersion: null, | ||
| emberDataVersion: null, | ||
| nodeJSVersion: nodeJSVersion |
There was a problem hiding this comment.
we could add broccoli-builder version here?
lib/broccoli-build-error.js
Outdated
| */ | ||
| this.broccoliPayload = { | ||
| versions: { | ||
| emberCLIVersion: null, |
There was a problem hiding this comment.
we should likely not suffix these with version
There was a problem hiding this comment.
also lets not camalize these.
instead:
versions: {
'broccoli-builder': ...
}that way its 1:1 with what all our version stuff (package.json etc, lockfiles use)
lib/builder.js
Outdated
| @@ -3,6 +3,9 @@ var mapSeries = require('promise-map-series') | |||
| var apiCompat = require('./api_compat') | |||
| var heimdall = require('heimdalljs'); | |||
| var SilentError = require('silent-error'); | |||
| var BroccoliBuildError = require('./broccoli-build-error'); | |||
|
|
|||
| var isThrown = false; | |||
There was a problem hiding this comment.
why is this module state and not builder instance state
|
released as v0.18.5 🎉 issue to remind us to forward port: #11 |
No description provided.