Added hook to validate if headings are present or not

[harish-sethuraman]

Addressing #4135 (comment) and followup for #4139

Going forward we will validate if the headings are present or not in the beta site's markdown files.

• If the headings are missing we will stop commit and show them an error message
• They can then generate the ids and then commit it

We will let the contributor decide which files to add and with pre commit hook we can make sure that we don't miss out any headings.

EDIT:
yarn lint-heading-ids --> Checks all files and causes an error if heading ID is missing
yarn lint-heading-ids --fix --> Fixes all markdown file's heading IDs
yarn lint-heading-ids path/to/markdown.md --> Checks that particular file for missing heading ID
(path can be a directory or particular file)
yarn lint-heading-ids --fix path/to/markdown.md --> Fixes that particular file's markdown IDs
(path can be a directory or particular file)

Making it a linter and added auto fixable option.

[github-actions]

Size Changes

📦 Next.js Bundle Analysis

This analysis was generated by the next.js bundle analysis action 🤖

This PR introduced no changes to the javascript bundle 🙌

[smikitky]

We could check only the staged files instead of the entire docs. I find this package useful (used in the Japanese fork to run additional linting for style problems):

https://www.npmjs.com/package/lint-staged

// package.json
{
  "dependencies": {
    "lint-staged": "^12.1.2"
  },
  "scripts": {
    "docs:validate-ids": "lint-staged"
  },
  "lint-staged": {
    "*.md": [
      "scripts/generateId.js" // this can either auto-add IDs or exit with an error
    ]
  }
}

[harish-sethuraman]

@smikitky Yea we could do that but consider someone force merges the markdowns without heading ID. we will not be able to generate headings for that later on since it won't be detected in other's commits (until they work on same file).

If we validate headings IDs only for staged files we should add a CI check for the PR to ensure we don't miss headings. (That will be added in all forked translation versions).

Should we validate headings only for staged files? and add a CI check ? Or rather would it make sense to validate heading IDs for all markdowns pre commit?

[smikitky]

@harish-sethuraman IMHO, ideally, it's best to have both a CI that checks everything and a pre-commit hook that checks (and optionally fixes) only the staged files. On second thought, however, at the moment, even prettier and eslint run on the entire codebase before every commit, so it may not make much sense to introduce lint-staged only around this topic...

What I don't like is that unrelated files may get committed without me noticing, so as long as that doesn't happen, I'm fine.

[harish-sethuraman]

Yep we need to have checks consistent. We have been running ESlint and prettier for the whole repo even in the old site.
If we want to introduce lint staged we can move all checks to lint staged at the same moment. Now shall we have checks run for all files as before?

PS: @smikitky wanna try adding validate heading id check for CI ?

@gaearon your thoughts ? Shall we move all checks to lint staged ?

[smikitky]

@harish-sethuraman I think we can just add the new docs:validate-ids command to this line:

https://github.com/reactjs/reactjs.org/blob/f625b86a48309a430b90d92d233d93c010de62da/beta/package.json#L17

Perhaps the hardest remaining task is getting validateHeadingIDs.js to accept file names as arguments.

[harish-sethuraman]

Perhaps the hardest remaining task is getting validateHeadingIDs.js to accept file names as arguments.

Yep we somehow have to figure out a way to validate headings only for staged files.

I think we can just add the new docs:validate-ids command to this line:

Added to CI check

[smikitky]

Whatever the policy will be, it would be very helpful to unify the scripts and having the API consistent with ESLint:

# checks all files and causes an error (process.exit(1)) if an ID is missing
# (currently validateHeadingIds)
lint-heading-ids.js

# checks all files and adds IDs if missing
# (currently generateHeadingIds)
lint-heading-ids.js --fix

# checks the specified files and causes an error if an ID is missing
# (used by hand or by lint-staged)
lint-heading-ids.js fileA.md path/to/fileB.md

# fix the specified files
# (used by hand or by lint-staged)
lint-heading-ids.js --fix fileA.md path/to/fileB.md

generateHeadingIds started as a one-off script to bulk-add IDs, but since I've seen the React team forget to add IDs many times, automating it would be great.

[harish-sethuraman]

@smikitky
yarn lint-heading-ids --> Checks all files and causes an error if heading ID is missing
yarn lint-heading-ids --fix --> Fixes all markdown file's heading IDs
yarn lint-heading-ids path/to/markdown.md --> Checks that particular file for missing heading ID
(path can be a directory or particular file)
yarn lint-heading-ids --fix path/to/markdown.md --> Fixes that particular file's markdown IDs
(path can be a directory or particular file)

Is this okay? These changes might help when we try to use lint staged as well

[smikitky]

@harish-sethuraman Yes the API looks perfect to me!

[gaearon]

seems fine, a few nits

[gaearon]

Why not fix them instead? Other scripts in this file fix things.

[harish-sethuraman]

• [Beta] Translating New React Docs #4135 (comment)

We did not want to fix the heading ids since other files without heading are modified while doing translations. Rather we wanted to just throw error so that we dont miss it in the main repo?