Introduce markdown linting (#2726)

* Introduce markdown lint * Add extension to devcontainer.json * Organize rule configuration * try introducing CI step * Configure formatter * Just add similar to eslint * update config * Fix: MD001/heading-increment/header-increment Heading levels should only increment by one level at a time * fix: MD030/list-marker-space Spaces after list markers * fix: MD040/fenced-code-language Fenced code blocks should have a language specified * Fix: MD051/link-fragments Link fragments should be valid * Create rules to enforce eventualy * Fix: no-emphasis-as-heading/no-emphasis-as-header Emphasis used instead of a heading * fix: MD031/blanks-around-fences Fenced code blocks should be surrounded by blank lines * Disable stylistic rule * Add TODO * Add note in config * fix: GH002/no-generic-link-text Avoid using generic link text like or : * update config * Add exceptions for Link for generic text * Update to @joshblack suggestion * Update CONTRIBUTING.md
primer · Jan 11, 2023 · 2e58630 · 2e58630
1 parent c0e9bb6
commit 2e58630
Show file tree

Hide file tree

Showing 33 changed files with 751 additions and 245 deletions.
diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json
@@ -1,13 +1,13 @@
 {
   "name": "Primer React",
   "image": "mcr.microsoft.com/vscode/devcontainers/typescript-node:16",
-  "extensions": ["esbenp.prettier-vscode", "dbaeumer.vscode-eslint"],
+  "extensions": ["esbenp.prettier-vscode", "dbaeumer.vscode-eslint", "DavidAnson.vscode-markdownlint"],
   "forwardPorts": [8000],
   "postCreateCommand": ["/bin/bash", "-c", "npm run setup"],
   "remoteUser": "node",
   "features": {
     "ghcr.io/devcontainers/features/sshd:1": {
-        "version": "latest"
+      "version": "latest"
     }
   },
   "hostRequirements": {

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -24,9 +24,11 @@ jobs:
       - name: Install dependencies
         run: npm ci
 
-      - name: Lint
+      - name: Lint JavaScript
         run: npm run lint
 
+      - name: Lint markdown
+        run: npm run lint:md
   test:
     strategy:
       fail-fast: false

diff --git a/.markdownlint-cli2.cjs b/.markdownlint-cli2.cjs
@@ -0,0 +1,35 @@
+const githubMarkdownOpinions = require('@github/markdownlint-github')
+
+// Rules we want to turn on but currently have too many violations
+const rulesToEnforce = {
+  'fenced-code-language': false,
+  'no-duplicate-header': false, // Fix https://github.com/primer/doctocat/issues/527, then set this rule to `siblings_only: true`
+}
+// Rules we don't care to enforce (usually stylistic)
+const rulesToNotEnforce = {
+  'line-length': false,
+  'blanks-around-headings': false,
+  'blanks-around-lists': false,
+  'no-trailing-spaces': false,
+  'no-multiple-blanks': false,
+  'no-trailing-punctuation': false,
+  'single-trailing-newline': false,
+  'ul-indent': false,
+  'no-hard-tabs': false,
+  'first-line-heading': false,
+  'no-space-in-emphasis': false,
+  'blanks-around-fences': false,
+}
+
+const defaultOverrides = {
+  'no-generic-link-text': {exceptions: ['link']}, // We don't want it to flag links that link to `Link` component.
+}
+
+const options = githubMarkdownOpinions.init({...rulesToNotEnforce, ...rulesToEnforce, ...defaultOverrides})
+module.exports = {
+  config: options,
+  customRules: ['@github/markdownlint-github'],
+  outputFormatters: [
+    ['markdownlint-cli2-formatter-pretty', {appendLink: true}], // ensures the error message includes a link to the rule documentation
+  ],
+}
diff --git a/README.md b/README.md
@@ -40,7 +40,7 @@ yarn add @primer/react
 
 ## Roadmap
 
-You can track our roadmap progress in the [Roadmap Project Board](https://github.com/primer/react/projects/3), see more detail in the [quarterly planning Discussions](https://github.com/primer/react/discussions?discussions_q=%5BRoadmap%5D), and find a list of all the current epic tracking issues [here](https://github.com/primer/react/discussions/997).
+You can track our roadmap progress in the [Roadmap Project Board](https://github.com/primer/react/projects/3), see more detail in the [quarterly planning Discussions](https://github.com/primer/react/discussions?discussions_q=%5BRoadmap%5D), and find a [list of all the current epic tracking issues](https://github.com/primer/react/discussions/997).
 
 ## Contributing
 
@@ -51,7 +51,3 @@ We love collaborating with folks inside and outside of GitHub and welcome contri
 ## New Component Proposals
 
 We welcome and encourage new component proposals from internal GitHub teams! Our best work comes from collaborating directly with the teams using Primer React Components in their projects. If you'd like to kick off a new component proposal, please submit an issue using the [component proposal issue template](https://github.com/primer/react/issues/new?template=new-component-proposal.md) and we will get in touch!
-
-[styled-components]: https://www.styled-components.com/docs
-[primer css]: https://github.com/primer/primer
-[flash of unstyled content]: https://en.wikipedia.org/wiki/Flash_of_unstyled_content
diff --git a/contributor-docs/CONTRIBUTING.md b/contributor-docs/CONTRIBUTING.md
@@ -85,7 +85,7 @@ const Component = styled.div<SxProp>`
 
 Component.defaultProps = {
   m: 0,
-  fontSize: 5
+  fontSize: 5,
 }
 
 export default Component
@@ -109,6 +109,8 @@ const Component = styled.div<SxProp>`
 
 ### Linting
 
+#### ESLint
+
 We use the [React configuration](https://github.com/github/eslint-plugin-github/blob/master/lib/configs/react.js) from [GitHub's eslint plugin](https://github.com/github/eslint-plugin-github) to lint our JavaScript. To check your work before pushing, run:
 
 ```sh
@@ -130,6 +132,14 @@ npm run lint -- --fix
 
 **Protip:** `npm run lint -- --quiet` (or `npx eslint --quiet ...`) will suppress warnings so that you can focus on fixing errors.
 
+#### Markdownlint
+
+We use [markdownlint](https://github.com/markdownlint/markdownlint) to lint Markdown files, using [GitHub's markdownlint-github configuration](https://github.com/github/markdownlint-github). To check your work before pushing, run:
+
+```sh
+npm run lint:md
+```
+
 ### TypeScript support
 
 Primer React is written in TypeScript. We include type definitions in our built artifacts. To check types, run the `type-check` test script:
@@ -212,10 +222,4 @@ Make sure to run `npm install` from inside the `docs/` subfolder _as well as_ th
 
 Ensure you are using the latest minor of Node.js for the major version specified in the `.nvmrc` file. For example, if `.nvmrc` contains `8`, make sure you're using the latest version of Node.js with the major version of 8.
 
-[classnames]: https://www.npmjs.com/package/classnames
-[spread syntax]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax
-[styled-system]: https://styled-system.com
-[table]: https://jxnblk.com/styled-system/table
-[npx]: https://www.npmjs.com/package/npx
-[now]: https://zeit.co/now
 [primer.style]: https://primer.style
diff --git a/contributor-docs/adrs/adr-004-children-as-api.md b/contributor-docs/adrs/adr-004-children-as-api.md
@@ -12,15 +12,16 @@ _Note: Consumer is used multiple times on this page. It refers to the developers
 
 ## Decision:
 
-
 1. Prefer using children for “content”
 
 2. For composite components, the API should be decided by how much customisation is available for children.
 
 For components that have design decisions baked in, should use strict props. For example, the color of the icon inside a Button component is decided by the `variant` prop on the Button. The API does not allow for changing that.
 
 ```jsx
-<Button variant="danger" leadingIcon={TrashIcon}>Delete branch</Button>
+<Button variant="danger" leadingIcon={TrashIcon}>
+  Delete branch
+</Button>
 ```
 
 On the other hand, if we want consumers to have more control over children, a composite API is the better choice.
@@ -38,7 +39,7 @@ On the other hand, if we want consumers to have more control over children, a co
 
 With React, `children` is the out-of-the-box way for putting [phrasing content](https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Content_categories#phrasing_content) inside your component. By using `children` instead of our own custom prop, we can make the API “predictable” for its consumers.
 
-<img width="373" alt="image" src="https://user-images.githubusercontent.com/1863771/144945223-70c4c800-5827-4985-9f18-0ab416eba058.png">
+<img width="373" alt="image" src="https://user-images.githubusercontent.com/1863771/144945223-70c4c800-5827-4985-9f18-0ab416eba058.png"> <!-- markdownlint-disable-line no-default-alt-text -->
 
 ```jsx
 // prefer this
@@ -62,7 +63,7 @@ import {CheckIcon} from '@primer/octicons-react'
 render(
   <Flash variant="success">
     <CheckIcon /> Changes saved!
-  </Flash>
+  </Flash>,
 )
 ```
 
@@ -149,8 +150,6 @@ When intentionally going off the happy path, developers can still drop down an a
 
 _Sidenote: We might want to name this prop `leadingIcon`, even if there is no `trailingIcon`. Consistent names across components plays a big role in making the API predictable._
 
-
-
 ---
 
 <br/>
@@ -233,7 +232,7 @@ We use this pattern as well in `Button`, `Button.Counter` is a restricted versio
 
 <img width="184" alt="image 8" src="https://user-images.githubusercontent.com/1863771/144945218-5154b8a1-8854-4335-926c-08a4ffac6d9d.png">
 
-```jsx
+````jsx
 <Button>
   Watch <Button.Counter>1</Button.Counter>
 </Button>
@@ -259,7 +258,7 @@ For Example, [legacy ActionMenu](https://primer.style/react/deprecated/ActionMen
 ```jsx
 <ActionMenu overlayProps={{width: 'medium'}} anchorContent="Open column menu">
 </ActionMenu>
-```
+````
 
 <br/>
 
@@ -371,15 +370,13 @@ Prefer using children for “content”
 <Button label="Watch" variant="primary"/>
 ```
 
-
 <img width="227" alt="image 13" src="https://user-images.githubusercontent.com/1863771/145045542-0d80491b-75e1-4304-b9fe-8c2cca80b298.png">
 
-
 The Icon should adapt to variant and size of the `Button`. We could use a `EyeIcon` in children here:
 
 ```jsx
 <Button>
-  <EyeIcon/> Watch
+  <EyeIcon /> Watch
 </Button>
 ```
 
@@ -393,10 +390,8 @@ But, we want to discourage customising the Icon’s color and size in the applic
 <Button leadingIcon={<EyeIcon/>}>Watch</Button>
 ```
 
-
 <img width="293" alt="image 14" src="https://user-images.githubusercontent.com/1863771/145045544-1a1651f1-fbcf-4022-8e9b-b37558bb2466.png">
 
-
 We want to add a `Counter` that adapts to the variant without supporting all the props of a `CounterLabel` like `scheme`.
 
 `Button.Counter` is a restricted version of `CounterLabel`, making the right thing easy and wrong thing hard:

diff --git a/contributor-docs/adrs/adr-005-box-sx.md b/contributor-docs/adrs/adr-005-box-sx.md
@@ -10,44 +10,44 @@ In Primer React and consuming applications, we use many different patterns for c
 
 1. Creating components with styled-components
 
-    ```tsx
-    const Avatar = styled.img.attrs<StyledAvatarProps>(props => ({
-      height: props.size,
-      width: props.size
-    }))<StyledAvatarProps>`
-      display: inline-block;
-      overflow: hidden;
-      line-height: ${get('lineHeights.condensedUltra')};
-      border-radius: ${props => getBorderRadius(props)};
-      ${sx}
-    `
-    ```
-
-    [Show full code example →](https://github.com/primer/react/pull/2019/files?diff=split&w=0)
-
-2.  Creating components with Box
-
-    ```tsx
-    const Avatar: React.FC<AvatarProps> = ({size = 20, alt = '', square = false, sx = {}, ...props}) => {
-      const styles:BetterSystemStyleObject = {
-        display: 'inline-block',
-        overflow: 'hidden',
-        lineHeight: 'condensedUltra',
-        borderRadius: getBorderRadius({size, square})
-      }
-
-      return (
-        <Box
-          as="img"
-          alt={alt}
-          sx={merge<BetterSystemStyleObject>(styles, sx)} // styles needs to merge with props.sx
-          {...props}
-        />
-      )
-    }
-    ```
-
-    [Show full code example →](https://github.com/primer/react/pull/2019/files?diff=split&w=0)
+   ```tsx
+   const Avatar = styled.img.attrs<StyledAvatarProps>(props => ({
+     height: props.size,
+     width: props.size,
+   }))<StyledAvatarProps>`
+     display: inline-block;
+     overflow: hidden;
+     line-height: ${get('lineHeights.condensedUltra')};
+     border-radius: ${props => getBorderRadius(props)};
+     ${sx}
+   `
+   ```
+
+   [Show full code example →](https://github.com/primer/react/pull/2019/files?diff=split&w=0)
+
+2. Creating components with Box
+
+   ```tsx
+   const Avatar: React.FC<AvatarProps> = ({size = 20, alt = '', square = false, sx = {}, ...props}) => {
+     const styles: BetterSystemStyleObject = {
+       display: 'inline-block',
+       overflow: 'hidden',
+       lineHeight: 'condensedUltra',
+       borderRadius: getBorderRadius({size, square}),
+     }
+
+     return (
+       <Box
+         as="img"
+         alt={alt}
+         sx={merge<BetterSystemStyleObject>(styles, sx)} // styles needs to merge with props.sx
+         {...props}
+       />
+     )
+   }
+   ```
+
+   [Show full code example →](https://github.com/primer/react/pull/2019/files?diff=split&w=0)
 
 &nbsp;
 

diff --git a/contributor-docs/adrs/adr-008-experimental-components.md b/contributor-docs/adrs/adr-008-experimental-components.md
@@ -8,7 +8,7 @@ Proposed
 
 ## Context
 
-#### Recap: Drafts
+### Recap: Drafts
 
 As we work on new components (or rewrites of old components), we often need to start with a "prototype"[^1] and not include them in the semantically versioned (semver-ed) main bundle.
 
@@ -39,8 +39,6 @@ The approach that has served us well since Dec 2021 has been "drafts" (along wit
 
    We want to enable feature teams to contribute to Primer. We also want to make it clear which contributions haven't been evaluated by our high standards for Primer components.
 
-
-
 &nbsp;
 
 ## Decision
@@ -53,17 +51,16 @@ The approach that has served us well since Dec 2021 has been "drafts" (along wit
 
 This will help in sharing experimental components between the monolith and projects outside of monolith. The ownership and responsibility of maintenance will be shared by multiple teams (including primer).
 
-#### Risks:
+### Risks:
 
 This will require improvements in the development and publishing workflow for experimental components. Without making that investment, we could create more friction and make contributions more difficult.
 
-
 &nbsp;
 
 #### Other options considered
 
-1. The code for experimental components should live in a new repository code `github.com/primer/react-candidates` to support different conventions and processes for experimental components and convey shared ownership between primer and product teams. 
- 
+1. The code for experimental components should live in a new repository code `github.com/primer/react-candidates` to support different conventions and processes for experimental components and convey shared ownership between primer and product teams.
+
    Keeping experimental components in primer _org_ suggests that the primer _team_ would take up maintenance of these components while they are still candidates (bugs, a11y remedial, etc.). This will be a new parallel workstream for the team and with our current team size, we might not be able to give it the required attention.
 
 2. The code for experimental components should live inside the monolith in [github/github/modules/react-shared](https://github.com/github/github/tree/master/app/assets/modules/react-shared) instead of a new repository.