-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
docs: validate that the docs can be parsed by mdx #2711
Conversation
|
||
class MarkdownError extends Error { | ||
constructor(file, inner) { | ||
super(`failed to parse ${file}`); |
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.
Do we care about the original error being eaten here?
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 a great use case for the error cause proposal, but lacking that, you could make MarkdownError extend AggregateError instead, and preserve the original error that way?
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.
@ljharb @wraithgar I don't think we care that much about this until we actually run into a problem making the docs pass/build. We can backlog some work to improve this & feel free to add changes.
ee7bfce
to
2d3c4ff
Compare
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.
LGTM
|
||
class MarkdownError extends Error { | ||
constructor(file, inner) { | ||
super(`failed to parse ${file}`); |
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.
@ljharb @wraithgar I don't think we care that much about this until we actually run into a problem making the docs pass/build. We can backlog some work to improve this & feel free to add changes.
Although our documentation is rendered for the "in the box" docs by cmark-gfm, the upstream docs site (docs.npmjs.com) uses mdx, which is a much stricter parser. Update our docs generator site to ensure that mdx can parse our documentation as well, to ensure that we get fast feedback when it would fail. PR-URL: npm#2711 Credit: @ethomson Close: npm#2711 Reviewed-by: @darcyclarke
2d3c4ff
to
af4422c
Compare
Although our documentation is rendered for the "in the box" docs by cmark-gfm, the upstream docs site (docs.npmjs.com) uses mdx, which is a much stricter parser. Update our docs generator site to ensure that mdx can parse our documentation as well, to ensure that we get fast feedback when it would fail.