[Markdown] Running Prettier

[wbamberg]

We should be running Prettier on our Markdown files to ensure that they are, well, pretty. I'm not very familiar with good practices here so I'm happy to get guidance. But to get things started, here is my understanding.

The h2m converter we use is already running Prettier, so what we start out with should be formatted properly. What we want is to ensure that it stays formatted properly without editors having to do this manually.

I think the approach is something like:

• add (a tool like) husky to mdn/content
• use it to define a pre-commit hook that runs Prettier

That way editors can write Markdown any way they like, and when they commit their changes are automatically reformatted.

Assuming that's the general approach, there are a couple of complexities:

Prettier configuration

First is Prettier configuration. I think there are two options we care about here:

Prose wrap

prose-wrap.

We want preserve here - it looks like this is the default for Markdown files?

Embedded language formatting

embedded-language-formatting.

This is a tricky one. We found that prettifying embedded HTML broke things (prettier/prettier#10950, mdn/content#5193 (comment)). So Gregor disabled it for everything except tables. Meaning that in the converter we pass embeddedLanguageFormatting: "off" (https://github.com/mdn/yari/blob/main/markdown/h2m/index.ts#L21), and handle HTML table formatting separately (https://github.com/mdn/yari/pull/4068/files#diff-49e0523c14ba735b440bb1bf76a8db617b0e534ce5f0f0f18d19344462aafd43). At least that's how I read this code!

So it seems like the best thing would be to disable embedded-language-formatting. This would mean we don't get formatting for HTML tables (which I'm not too worried about). But from the Prettier docs it seems like we also wouldn't get it for code blocks, which would be a real shame.

I just experimented using my editor's Prettier plugin, and it does format code blocks but doesn't format embedded HTML, which seems to be what we would like. But I don't know if it would work the same using the Prettier CLI.

markdown-lint

We would also like to run markdown-lint on our source. I guess the general approach would be the same (pre-commit hooks). But we would need to disable any rules that conflict with Prettier.

[ddbeck]

Overall approach

I think the approach is something like:

• add (a tool like) husky to mdn/content

• use it to define a pre-commit hook that runs Prettier

That way editors can write Markdown any way they like, and when they commit their changes are automatically reformatted.

I think this approach will only be part of the story. I expect a lot of contributions to come through GitHub's web editor, where commit hooks won't happen (perhaps a lot more contributions, even, once we're in Markdown and editing becomes easier).

Also, curmudgeons like me who come with their own byzantine Git habits will bypass Husky, too. I'd prefer for hooks to be opt-in rather than opt-out, to be honest; my experience with Husky has always been slow and distracting.

Instead, I think we'll probably want a couple different tools to help apply formatting consistently:

• CI to prevent merges that fail Prettier's --check option
• Git hooks for those who want them
• A GitHub Action to apply formatting to PRs (i.e., a reviewer can just request an automatic application of Prettier to a PR that was created with the GitHub editor)

Formatting code blocks

So it seems like the best thing would be to disable embedded-language-formatting. This would mean we don't get formatting for HTML tables (which I'm not too worried about). But from the Prettier docs it seems like we also wouldn't get it for code blocks, which would be a real shame.

I wonder if we could override embedded languages selectively? Perhaps we could sub in no-op parser for HTML only. Rooting around in the Prettier issues, it looks like some projects have done something like this (prettier/prettier#5919 has some interesting discussion here; seems that WordPress has done a minor fork of Prettier, which may be inspirational).

[ddbeck]

Since this came up in the most recent editorial call, I promised to expand on this a little bit. Generally, I think you need to be able to apply Prettier (and/or markdownlint) at various layers. Think of it as a sort of defense in depth approach:

1. A prettier config (e.g., a config file or the package.json key-value) to set what formatted means for the project in general.

2. A GitHub Actions workflow to require that Prettier and markdownlint pass for the modified files in the PR.

   This gets you the pass/fail indicator on the PR.

3. A GitHub Actions workflow to automatically apply Prettier or markdownlint fixes to a PR.

   For example, adding a format-needed label or commenting with /format could trigger a workflow that adds a commit to the PR, which applies the fixes, and, on success, removes the label or comment. Then a reviewer can proceed with a complete review.

4. Add text editor configs to help editors get the correct formatting by default (e.g., VS Code's recommended extensions)

5. Add optional pre-commit hooks to help editors get the correct formatting by default.

You could probably ship Prettier formatting with the first two or three and finish the rest in flight. mdn/content#10402 outlines this approach for Prettier. I imagine something similar could be adapted for markdownlint.

[hamishwillee]

I'd be OK with pretiffying being done in CI provided we trusted it only happen on the final merge into main.
If done on every commit to a PR we might well end up with cases where I submit my commit, it gets "fixed" by CI, and when I next push up a commit there is a conflict with that fixed up code. Horrible.

If not done in the final step, my preference would be that prettify is something I do in yari OR I can trigger in the PR. CI could still report the need to run prettifier, but at least I would choose when this happens and it wouldn't conflict with a normal editorial workflow.

[nschonni]

FYI, I'd suggest https://www.npmjs.com/package/lint-staged over plain Husky, since then the linter (prettier, Markdownlint, yari, etc...) only executes on the changed files, instead of a slower crawl on everything. The big husky hooks are usually when I start to skip them like @ddbeck

[OnkarRuikar]

If we don’t want to burden the contributors(online GitHub) then is it possible to implement following things?

1. Add lint suggestions to the PR companion(github-action bot) comment. It’ll be up to the contributor or reviewer to implement or enforce it depending on the severity.
2. Or add lint suggestions as a review to the PR.
3. Run automation on every Saturday which will look for the files modified that week and run prettier(and other cleanups) and submit a PR. By Monday someone will merged it. And we’ll start the week with clean content.

[hamishwillee]

IMO

1. Whatever else we do, option "3" sounds like a good action. At least that prevents things getting out of hand. Though perhaps a number of PRs split across main areas.
2. I quite like adding suggestions to the PR companion bot comment - but it would be good if the author or reviewer could have an easy way to trigger accepting the suggestions - i.e. a link or button.

[nschonni]

Cleanup automation in mdn/content#16036. I left it as daily, since it can just keep going till a maintainer merges it. If there are now changes, then it won't create a PR for that day

[bsmth]

I think we can mark this as done given all of the work that has been completed on this.

Here are some highlights:

• Format all MDN content with Prettier mdn#362
• chore: Add Husky/lint-staged for local precommit content#20397
• run prettier on .md files in precommit hook content#27936
• feat(ci): provide automated PR reviews for linting related changes content#28363
• feat(ci): Add Reviewdog for Prettier formatting content#28228

Well done, everybody!