docs: validate that the docs can be parsed by mdx by ethomson · Pull Request #2711 · npm/cli

ethomson · 2021-02-16T18:59:05Z

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.

wraithgar · 2021-02-16T19:18:29Z

+
+class MarkdownError extends Error {
+ constructor(file, inner) {
+ super(`failed to parse ${file}`);


Do we care about the original error being eaten here?

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?

@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.

darcyclarke

LGTM

darcyclarke · 2021-02-18T19:27:00Z

+
+class MarkdownError extends Error {
+ constructor(file, inner) {
+ super(`failed to parse ${file}`);


@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.

@ethomson

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: @ethomsonClose: npm#2711 Reviewed-by: @darcyclarke

ethomson · 2021-02-16T18:59:05Z

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.

wraithgar · 2021-02-16T19:18:29Z

+
+class MarkdownError extends Error {
+ constructor(file, inner) {
+ super(`failed to parse ${file}`);


Do we care about the original error being eaten here?

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?

@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.

darcyclarke

LGTM

darcyclarke · 2021-02-18T19:27:00Z

+
+class MarkdownError extends Error {
+ constructor(file, inner) {
+ super(`failed to parse ${file}`);


@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.

@ethomson

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: @ethomsonClose: npm#2711 Reviewed-by: @darcyclarke

ethomson · 2021-02-16T18:59:05Z

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.

wraithgar · 2021-02-16T19:18:29Z

+
+class MarkdownError extends Error {
+ constructor(file, inner) {
+ super(`failed to parse ${file}`);


Do we care about the original error being eaten here?

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?

@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.

darcyclarke

LGTM

darcyclarke · 2021-02-18T19:27:00Z

+
+class MarkdownError extends Error {
+ constructor(file, inner) {
+ super(`failed to parse ${file}`);


@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.

@ethomson

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: @ethomsonClose: npm#2711 Reviewed-by: @darcyclarke

ethomson · 2021-02-16T18:59:05Z

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.

wraithgar · 2021-02-16T19:18:29Z

+
+class MarkdownError extends Error {
+ constructor(file, inner) {
+ super(`failed to parse ${file}`);


Do we care about the original error being eaten here?

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?

@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.

darcyclarke

LGTM

darcyclarke · 2021-02-18T19:27:00Z

+
+class MarkdownError extends Error {
+ constructor(file, inner) {
+ super(`failed to parse ${file}`);


@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.

@ethomson

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: @ethomsonClose: npm#2711 Reviewed-by: @darcyclarke

ethomson · 2021-02-16T18:59:05Z

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.

wraithgar · 2021-02-16T19:18:29Z

+
+class MarkdownError extends Error {
+ constructor(file, inner) {
+ super(`failed to parse ${file}`);


Do we care about the original error being eaten here?

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?

@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.

darcyclarke

LGTM

darcyclarke · 2021-02-18T19:27:00Z

+
+class MarkdownError extends Error {
+ constructor(file, inner) {
+ super(`failed to parse ${file}`);


@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.

@ethomson

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: @ethomsonClose: npm#2711 Reviewed-by: @darcyclarke

ethomson · 2021-02-16T18:59:05Z

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.

wraithgar · 2021-02-16T19:18:29Z

+
+class MarkdownError extends Error {
+ constructor(file, inner) {
+ super(`failed to parse ${file}`);


Do we care about the original error being eaten here?

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?

@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.

darcyclarke

LGTM

darcyclarke · 2021-02-18T19:27:00Z

+
+class MarkdownError extends Error {
+ constructor(file, inner) {
+ super(`failed to parse ${file}`);


@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.

@ethomson

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: @ethomsonClose: npm#2711 Reviewed-by: @darcyclarke

ethomson · 2021-02-16T18:59:05Z

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.

wraithgar · 2021-02-16T19:18:29Z

+
+class MarkdownError extends Error {
+ constructor(file, inner) {
+ super(`failed to parse ${file}`);


Do we care about the original error being eaten here?

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?

@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.

darcyclarke

LGTM

darcyclarke · 2021-02-18T19:27:00Z

+
+class MarkdownError extends Error {
+ constructor(file, inner) {
+ super(`failed to parse ${file}`);


@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.

@ethomson

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: @ethomsonClose: npm#2711 Reviewed-by: @darcyclarke

ethomson · 2021-02-16T18:59:05Z

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.

wraithgar · 2021-02-16T19:18:29Z

+
+class MarkdownError extends Error {
+ constructor(file, inner) {
+ super(`failed to parse ${file}`);


Do we care about the original error being eaten here?

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?

@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.

darcyclarke

LGTM

darcyclarke · 2021-02-18T19:27:00Z

+
+class MarkdownError extends Error {
+ constructor(file, inner) {
+ super(`failed to parse ${file}`);


@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.

@ethomson

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: @ethomsonClose: npm#2711 Reviewed-by: @darcyclarke

ethomson requested a review from a team as a code owner February 16, 2021 18:59

wraithgar reviewed Feb 16, 2021
View reviewed changes

ethomson force-pushed the ethomson/docs-mdx-validation branch from ee7bfce to 2d3c4ffCompare February 16, 2021 19:19

darcyclarke added Release 7.x work is associated with a specific npm 7 release release: next These items should be addressed in the next release semver:patch semver patch level for changes labels Feb 17, 2021

darcyclarke approved these changes Feb 18, 2021
View reviewed changes

nlf changed the base branch from latest to release/v7.5.5February 18, 2021 20:49

nlf force-pushed the ethomson/docs-mdx-validation branch from 2d3c4ff to af4422cCompare February 18, 2021 20:49

nlf merged commit af4422c into npm:release/v7.5.5Feb 18, 2021

nlf mentioned this pull request Feb 22, 2021
Release/v7.5.5 #2754
Merged

ruyadorno mentioned this pull request Feb 24, 2021
deps: upgrade npm to 7.5.6 nodejs/node#37496
Closed

snyk-ioBot mentioned this pull request Oct 21, 2024
[Snyk] Fix for 28 vulnerabilities Graybar-codespace/cli-3#27
Open