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

Docusaurus version bump to warn about broken anchors; other fixes #2578

Merged
merged 4 commits into from
Jun 4, 2024

Conversation

lukesonnet
Copy link
Collaborator

@lukesonnet lukesonnet commented May 29, 2024

There were about ~10 broken anchor links and 1 broken import that Docusaurus was not waring about before.

New versions check import statements and anchors more strictly, so now we can easily catch and fix broken anchor links.

Currently set to "warn" instead of "throw" for broken anchor links since links to the api docs are broken until (I think) this issue is closed: rohit-gohri/redocusaurus#321

Copy link

github-actions bot commented May 29, 2024

Deploy preview for docs ready!

✅ Preview
https://docs-p9xhktcxr-growthbook.vercel.app

Built with commit 2936e52.
This pull request is being automatically deployed with vercel-action

@lukesonnet lukesonnet changed the title Add explicit multiple exposures section to troubleshooting doc Docusaurus version bump to warn about broken anchors; other fixes May 30, 2024
Copy link
Contributor

@romain-growthbook romain-growthbook left a comment

Choose a reason for hiding this comment

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

This looks good if you've tested it. Do we expose the broken link check somewhere? Could a check be integrated into the CI?

@@ -0,0 +1,8 @@
import React from "react";
// eslint-disable-next-line import/no-unresolved
import useBrokenLinks from "@docusaurus/useBrokenLinks";
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks great. Do you know why the module is reported as unresolved?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No I don't but I see we've done this elsewhere in this folder so I didn't interrogate it at the time.

@@ -6,7 +6,7 @@ slug: amplitude
---

import DefaultInstructions from './_default-event-instructions.mdx'
const { title, description, sidebar_label, slug } = frontMatter
export const { title, description, sidebar_label, slug } = frontMatter
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Do those need to be exported? They don't seem to be consumed anywhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the mdx to js approach we use expects it to be exported (it was the library complaining about these not being exported), presumably because they do get used somewhere else once the files are compiled. I'm not sure. Here's their docs: https://mdxjs.com/docs/using-mdx/

@romain-growthbook
Copy link
Contributor

My bad, just reading your initial description. Ideally, we should make sure that we error when new broken link are discovered. Not sure how practical this would be.

@lukesonnet
Copy link
Collaborator Author

My bad, just reading your initial description. Ideally, we should make sure that we error when new broken link are discovered. Not sure how practical this would be.

I agree, I just don't know if we can "ignore" these false positives in the .mdx files (I couldn't find a way) so we have to set the level to warn until the bug is resolved.

@lukesonnet lukesonnet merged commit 1e8a9e4 into main Jun 4, 2024
4 checks passed
@lukesonnet lukesonnet deleted the ls/trouble-doc branch June 4, 2024 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redocusaurus should collect heading ids / anchors
2 participants