-
-
Notifications
You must be signed in to change notification settings - Fork 597
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
feat(json): log the filename when JSON.parse fails #417
Conversation
78db740
to
0d3c00e
Compare
795fbcb
to
0efe962
Compare
This should be ready to go now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good for this PR, but before we publish this change I want to utilize a custom error class and do some more maintenance on this plugin. Changing Error expectations means a major version, so it's a good time to tackle that.
packages/json/src/index.js
Outdated
} catch (err) { | ||
const error = new Error(`Could not parse ${id}`); | ||
error.originalError = err; | ||
throw error; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I sat on this for a while and I'm glad that I did. I'd like to see this use this.warn
rather than throwing the error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Helpful when debugging Sometimes in a large codebase, a single comma buried in a gigantic json file can halt the entire build. This change ensures that users will more easily be able to find and fix those errors.
Helpful when debugging Sometimes in a large codebase, a single comma buried in a gigantic json file can halt the entire build. This change ensures that users will more easily be able to find and fix those errors.
Rollup Plugin Name:
json
This PR contains:
Are tests included?
Breaking Changes?
If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.
List any relevant issue numbers:
Description
Helpful when debugging
Sometimes in a large codebase, a single comma buried in a gigantic json file can halt the entire build. This change ensures that users will more easily be able to find and fix those errors.