Skip to content

[docs-infra] Fix Vale no longer working #46029

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

Merged
merged 6 commits into from
Apr 30, 2025

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Apr 27, 2025

I was working on a quick fix, mui/mui-x#17538. There I noticed in MUI X that Vale was reporting 100 errors https://github.com/mui/mui-x/actions/runs/14686411363/job/41215568904.

SCR-20250427-mnmz

but it was strange as I don't get those in Material UI nor if I run the script locally. However, in Material UI, it just crashes: https://github.com/mui/material-ui/actions/runs/14682247158/job/41206345211

SCR-20250427-mnta

as it turned out, it was a pile of problems, and broken for some time:

  1. The markdown yaml headers we have are not valid YAML, can be verified in https://www.yamllint.com/. It needs to be properly escaped. A quick intro in https://symfony.com/doc/current/reference/formats/yaml.html#strings.
  2. But it also means that the conversion logic needs to be fixed to support it.
  3. We have no versioning on the version of vale that the GitHub action runs. It's "latest". Let's force a version, using https://github.com/errata-ai/vale-action?tab=readme-ov-file#version-default-latest.
  4. Now, with this fixed, we can reproduce the 100 errors. I have fixed this by having the substation match the spacing.
  5. It seems that we have 4 versions of Vale to sync npm package errata-ai/vale#829 (comment). Starting to use an npm version to more easily version and reproduce result between envs.
  6. Rules started to diverge between Material UI and MUI X, because it was broken in X we didn't saw it until now, fixed the charts rules.

🧹 done. The last thing would be to have 1. the Vale VS Code extension use the binary available locally 2. an official npm version, maybe one day.


Next:

  • Update each repo. Update .github/workflows/vale-action.yml, update package.json, update pnpm-workspace.yaml.

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work regression 🐛 A bug, but worse scope: docs-infra Specific to the docs-infra product labels Apr 27, 2025
@mui-bot
Copy link

mui-bot commented Apr 27, 2025

Netlify deploy preview

https://deploy-preview-46029--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 914eb78

@oliviertassinari oliviertassinari force-pushed the fix-vale branch 6 times, most recently from 8f4440d to 88641be Compare April 27, 2025 19:44
@oliviertassinari oliviertassinari mentioned this pull request Apr 27, 2025
1 task
@oliviertassinari oliviertassinari removed the bug 🐛 Something doesn't work label Apr 28, 2025
Comment on lines +24 to +26
Sparkline Chart: Sparkline chart
Gauge Chart: Gauge chart
Treemap Chart: Treemap chart
Copy link
Member

Choose a reason for hiding this comment

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

Those one are correct. we agree on the fact that those item does not need an extra "chart"

Suggested change
Sparkline Chart: Sparkline chart
Gauge Chart: Gauge chart
Treemap Chart: Treemap chart
Sparkline Chart: Sparkline
Gauge Chart: Gauge
Treemap Chart: Treemap

Copy link
Member Author

@oliviertassinari oliviertassinari Apr 29, 2025

Choose a reason for hiding this comment

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

I did the change because of https://github.com/mui/mui-x/pull/17602/files#r2067364155. It doesn't seem that we can do anything about it. We have to allow it.

@@ -1,5 +1,5 @@
---
title: 'The 2019 Material UI Developer Survey: here's what we discovered'
title: "The 2019 Material UI Developer Survey: here's what we discovered"
Copy link
Member

Choose a reason for hiding this comment

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

Is their a specific reason why using " instead of ' ?

Copy link
Member Author

@oliviertassinari oliviertassinari Apr 29, 2025

Choose a reason for hiding this comment

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

The other valid YAML syntax would be:

Suggested change
title: "The 2019 Material UI Developer Survey: here's what we discovered"
title: 'The 2019 Material UI Developer Survey: here''s what we discovered'

I was lazy, didn't add support for the '' escaping.

Comment on lines 89 to 91
components: Alert, AlertTitle
githubLabel: 'component: alert'
packageName: '@mui/lab'
waiAria: https://www.w3.org/TR/wai-aria-practices/#alert
authors:
['foo', 'bar']
Copy link
Member

Choose a reason for hiding this comment

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

Why removing those lines?

Copy link
Member Author

Choose a reason for hiding this comment

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

The goal is to keep the test more focused on what we try to test, this is unrelated.

@alexfauquette
Copy link
Member

I've open a PR to test it on MUI X mui/mui-x#17602

@oliviertassinari oliviertassinari merged commit 1d1746d into mui:master Apr 30, 2025
24 checks passed
@oliviertassinari oliviertassinari deleted the fix-vale branch April 30, 2025 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression 🐛 A bug, but worse scope: docs-infra Specific to the docs-infra product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants