-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Adds error message with filename on Markdown error, fixes bug in panicOnBuild #8866
Conversation
createParentChildLink({ parent: node, child: markdownNode }) | ||
} catch (err) { | ||
reporter.panicOnBuild( | ||
`Error processing Markdown file ${node.absolutePath}:\n |
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 forgot about something earlier - markdown doesn't always come from file, so we will need to check if node.absolutePath
is set before using it. For example:
`Error processing Markdown ${node.absolutePath ? `file: ${node.absolutePath}` : ``}:\n`
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.
Looking good, only one last small thing to fix (my bad for not thinking about it earlier when I left some comments in the issue)
Made the word 'file' conditional as well
@pieh Thanks for pointing that out – hadn't considered that. Changed it up a bit so it checks if the path is available. |
What about giving some fallback identifier like node.id? This could help to find the error if the markdown isn't a file. |
It definitely won't hurt. |
Cool, can show |
BTW, you could also on the fly start using the new |
Let's not derail this PR into adding more stuff here :)
|
As discussed in the PR, it might help people to show the node id when no absolutePath is available (e.g. when not using a file to supply the Markdown)
@pieh Great, thanks! Added the change to show the node.id. |
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.
Thanks @mslooten!
Holy buckets, @mslooten — we just merged your PR to Gatsby! 💪💜 Gatsby is built by awesome people like you. Let us say “thanks” in two ways:
If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’. Thanks again! |
As discussed in #8524, when Markdown isn't correctly processed an error is thrown which doesn't include the filename. This fixes it using a try/catch with the
reporter.panicOnBuild
function, which is also changed since it had unexpected behavior (it didn't terminate the build, but it did terminate on other commands such asgatsby develop
).---edit by @pieh:
closes #8524