Skip to content

Conversation

@oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Oct 30, 2024

A quick fix for a regression that I noticed while fixing mui/material-ui#44277. We were no longer linting the docs. This got broken with #538.

Ideally, we could use errata-ai/vale#841, but it's not there. Instead, there are a bunch of examples online on how this can be done https://github.com/search?q=%5Bformats%5D+mdx+%3D+md&type=code.

We will need to propagate this to the other repositories. But unclear if we should do it just yet, we might want to wait.
Point 20. of #588 doesn't make it super clear that MDX is compatible with a great UX for developers. I imagine that it's solvable, but it's it's not it seems to mean that we need to remove MDX and go back to the previous approach that yields better performance.

@oliviertassinari oliviertassinari added type: bug It doesn't behave as expected. type: regression A bug, but worse, it used to behave as expected. labels Oct 30, 2024
@mui-bot
Copy link

mui-bot commented Oct 30, 2024

Netlify deploy preview

https://deploy-preview-769--base-ui.netlify.app/

Generated by 🚫 dangerJS against b18d8d7

Copy link
Collaborator

@vladmoroz vladmoroz left a comment

Choose a reason for hiding this comment

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

Point 20. of #588 doesn't make it super clear that MDX is compatible with a great UX for developers. I imagine that it's solvable, but it's it's not, it seems to mean that we need to remove MDX and go back to the previous approach that yields better performance.

I'll look into it, seems that we'd need to lean in more into server components there to fix it instead of passing around the code snippets in React context.

Compressed it's about the same though, comparing the dialog pages:

Screenshot 2024-10-30 at 17 20 27

Screenshot 2024-10-30 at 17 20 30

Neither old or new docs get stellar ratings on PageSpeed Insights too:

Screenshot 2024-10-30 at 17 30 15

Screenshot 2024-10-30 at 17 30 05

We should be able to do better though with just a docs website.

[formats]
mdx = md

[*.md]
Copy link
Member Author

Choose a reason for hiding this comment

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

This is kind of magic, it lint mdx too, it doesn't need to be:

Suggested change
[*.md]
[*.{md,mdx}]

so I didn't change it 🤷‍♂️

@michaldudak
Copy link
Member

Point 20. of #588 doesn't make it super clear that MDX is compatible with a great UX for developers.

This isn't caused by MDX, but the Next.js App Router.

@oliviertassinari oliviertassinari merged commit 48c675b into mui:master Nov 1, 2024
18 checks passed
@oliviertassinari oliviertassinari deleted the mdx-vale branch November 1, 2024 22:34
@oliviertassinari oliviertassinari removed the type: bug It doesn't behave as expected. label Apr 30, 2025
@michaldudak michaldudak added the scope: code-infra Involves the code-infra product (https://www.notion.so/mui-org/5562c14178aa42af97bc1fa5114000cd). label May 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: code-infra Involves the code-infra product (https://www.notion.so/mui-org/5562c14178aa42af97bc1fa5114000cd). type: regression A bug, but worse, it used to behave as expected.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants