[Tracking] Enforce a source style in Markdown files with Prettier

[ddbeck]

This is a meta issue for the work that needs to happen to adopt to an automated, formalized code style for MDN content. This overall approach was proposed in https://github.com/mdn/content/discussions/9901#discussioncomment-1576438 and wasn't objected to, so this is how we're doing it.

MDN has several guidelines for code style. See Code example guidelines. Instead of asking people to know and carry out a style, we ought to do the modern thing: directly format what we want.

Distinct slices of work are broken down in the following issues:

• [Prettier] Refer readers to Prettier style in the JavaScript guidelines #10403 - Refer readers to Prettier style in the guidelines and advise readers that we'll reject any style-only PRs
• [Prettier] Enforce Prettier in CI #10411 (blocked by descendants)
  • [Prettier] Create automation for reviewers to apply code format to PRs #10406
    • [Prettier] Add an initial Prettier config to the repo #10405

[wbamberg]

Thanks for getting this started Daniel!

Is it worth distinguishing between (and possibly having separate stages for) (1) using Prettier to format Markdown, and (2) using Prettier to format embedded code?

(1) can be all about integrating Prettier into the workflow: we should already have Prettier-formatted Markdown as a result of the conversion process, so it should not be very disruptive to the content.

(2) has some extras challenges:

• it will be quite disruptive to the content
• we need to decide how to stop Prettier fixing intentionally bad code samples
• we have to decide whether to care about embedded code samples that are a copy of some code living somewhere else. For instance the Learn area code samples are almost always shadowed by code in https://github.com/mdn/learning-area. If we Prettify mdn/content, we should also Prettify mdn/learning-area (and mdn/css-examples, and...)

[ddbeck]

@wbamberg You've raised some issues I carefully avoided thinking too hard about. Good job!

When I started writing the issues, I wasn't fully decided on the Markdown versus embedded code question. But the reading linked from #10411 was pretty instructive. I think we should format whole files as we're able to, rather than partly formatting in multiple passes. I imagine there are many files where we don't have to deal with code at all or the code doesn't raise any complicated issues (e.g., glossary entries). We ought to get the benefit in those files early, rather than waiting until we have a solution for every case.

Primarily, I want to avoid some of the headaches of Markdown, where we caused mass PR invalidations. That was worth the cost, but I'm not sure we get the same gains from Prettier. Instead, I want to be able to ratchet up our formatting over time (before eventually switching tracks to get us to 100%).

To some more specific points:

we need to decide how to stop Prettier fixing intentionally bad code samples

I think this can be done with pragmas or a no-op language identifier in the info line for code blocks (i.e., ```mdn-bad-example). This specific thing feels tractable. I imagine there is non-JS and non-bad-example code that may test us though.

we have to decide whether to care about embedded code samples that are a copy of some code living somewhere else. For instance the Learn area code samples are almost always shadowed by code in https://github.com/mdn/learning-area. If we Prettify mdn/content, we should also Prettify mdn/learning-area (and mdn/css-examples, and...)

This is not something I had given any thought about! I guess this suggests that we should make sure the config, Actions workflows, etc. are portable, so it's more of a copy-and-paste operation to those places.

I'm not quite sure what to do with these yet. We don't have the pieces in place to apply Prettier yet, so I'm not sure I should open issues for "figure out how to apply Prettier to …" until we have concrete examples of things we would like to format, but can't. Though maybe this is just ignorance on my part—if you think some of this is already well-defined, I'd welcome more issues to hang off of this one.

[wbamberg]

Primarily, I want to avoid some of the headaches of Markdown, where we caused mass PR invalidations.

It caused a rather small number of PR invalidations, mostly to PRs which were moribund, so would have been closed sooner or later anyway. We managed to merge almost all PRs for which the author was responsive. So honestly I don't think this is a huge problem - mostly because do well at keeping the PR backlog under control.

It would be great to avoid them of course, but doing that comes with its own cost in terms of complexity and time.

(For example, I could have opted to split the Web/API conversion into N pieces. But that would have taken much longer and given us N chances to make a mistake. Although doing everything in one go was pretty stressful, I'm glad we did it that way.)

In the "Facebook Adoption Update" link it says:

Then, Oculus and Nuclide converted their codebase over. The scale is bigger with a few thousands of files and tens of full time contributors but looks pretty similar to the first projects. The conversions went in one big codemod and that's it.

In scale this doesn't seem that different to mdn/content.

But: I'm not likely to be managing this project, so it's not really my concern how it is managed!

[ddbeck]

It caused a rather small number of PR invalidations

Ah, interesting! I misunderstood the whole situation where we locked merging temporarily. I thought it was a bigger problem than it actually was. If we don't have to do it incrementally, then that's good! Though I suppose having the option will be nice.

[hamishwillee]

Excellent plan. For #10411 I think there was a discussion somewhere of providing an action that could be used to manually trigger prettification from github if required, to make it easy for reviewers to unblock PRs.

[ddbeck]

@hamishwillee I created #10406 to cover that specifically. 👍

[nschonni]

Think on first step could be to do a find/replace for

---
{{

with

---

{{

This seems to be a common change from prettier in most files, since it wants a space after the front matter.

I've run it on a few folders, but this one change is needed on ~9K files, so it would probably need to be done in some batch sizes and without prettier so it wouldn't introduce any other changes

[teoli2003]

Yes, this would be nice. Once this change is done, running Prettier will lead to much smaller diffs.

If this fix is the only change, batches can be fairly large. I would say 1 per top-level directory, except for web/ where one per second-level directory (web/css, …) would be nice, with a cap to 1000 (for web/api). These are easy to review.

I'll happily approve them.

[nschonni]

LOL, hit this on the last PR from VSCode GraphQL error: API rate limit exceeded for user ID 1297909.

[nschonni]

So after the pass for that metadata spacing, running prettier now changes 7900 files in the repo.
I realized that not all the metadata sections have {{ immediately after, so I might try for another pass to see if I can get more of those metadata block spacing

[Josh-Cena]

We already have Prettier and CI. There is some lingering work but that can be completed without keeping this issue open.