Skip to content

[Markdown] Running Prettier #8729

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

Closed
wbamberg opened this issue Sep 7, 2021 · 2 comments
Closed

[Markdown] Running Prettier #8729

wbamberg opened this issue Sep 7, 2021 · 2 comments
Labels
needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened.

Comments

@wbamberg
Copy link
Collaborator

wbamberg commented Sep 7, 2021

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, #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.

@wbamberg wbamberg added the needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. label Sep 7, 2021
@ddbeck
Copy link
Contributor

ddbeck commented Sep 9, 2021

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).

@hamishwillee
Copy link
Collaborator

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.

@mdn mdn locked and limited conversation to collaborators Sep 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened.
Projects
None yet
Development

No branches or pull requests

3 participants