Skip to content
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

Fix MDX stack trace location with frontmatter #11717

Merged
merged 1 commit into from
Aug 16, 2024
Merged

Fix MDX stack trace location with frontmatter #11717

merged 1 commit into from
Aug 16, 2024

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Aug 15, 2024

Changes

fix #11707

When we extract the frontmatter with gray-matter and it returns the content, it doesn't include the new lines from the stripped frontmatter, causing the MDX parser to incorrectly pinpoint the location of the error.

This PR manually adds the new lines to preserve the location.

Testing

Tested manually. The location is now correct:
image

Docs

n/a. bug fix.

Copy link

changeset-bot bot commented Aug 15, 2024

🦋 Changeset detected

Latest commit: b2d30eb

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: integration Related to any renderer integration (scope) label Aug 15, 2024

const vfile = new VFile({ value: pageContent, path: id });
const vfile = new VFile({ value: frontmatterLines + pageContent, path: id });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any risk of this being breaking?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, it's only prepending new lines. Do you have something in mind that could break?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured that perhaps a remark/hype plugin could be relying on certain positions, but I guess that's unlikely

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I guess you're right since the mdast/hast nodes do have offset information, but since they only work upon the vfile.value, it should still be fine. Just that if they return any offset values then it'd be incorrect, but I don't think you'd do so in practice, except in the linked issue where the returned error uses line/col instead of offsets.

We could replace the frontmatter with spaces and new lines, but I think maybe for now new lines could be enough and we can monitor it again.

@bluwy bluwy merged commit 423614e into main Aug 16, 2024
13 checks passed
@bluwy bluwy deleted the fix-mdx-loc branch August 16, 2024 04:09
@astrobot-houston astrobot-houston mentioned this pull request Aug 16, 2024
akshit20421 added a commit to akshit20421/astro that referenced this pull request Aug 16, 2024
Fix MDX stack trace location with frontmatter (withastro#11717)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: integration Related to any renderer integration (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MDX with frontmatter reports errors on wrong line
2 participants