Skip to content

Commit 2e58630

Browse files
authored
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
1 parent c0e9bb6 commit 2e58630

33 files changed

+751
-245
lines changed

.devcontainer/devcontainer.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
{
22
"name": "Primer React",
33
"image": "mcr.microsoft.com/vscode/devcontainers/typescript-node:16",
4-
"extensions": ["esbenp.prettier-vscode", "dbaeumer.vscode-eslint"],
4+
"extensions": ["esbenp.prettier-vscode", "dbaeumer.vscode-eslint", "DavidAnson.vscode-markdownlint"],
55
"forwardPorts": [8000],
66
"postCreateCommand": ["/bin/bash", "-c", "npm run setup"],
77
"remoteUser": "node",
88
"features": {
99
"ghcr.io/devcontainers/features/sshd:1": {
10-
"version": "latest"
10+
"version": "latest"
1111
}
1212
},
1313
"hostRequirements": {

.github/workflows/ci.yml

+3-1
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,11 @@ jobs:
2424
- name: Install dependencies
2525
run: npm ci
2626

27-
- name: Lint
27+
- name: Lint JavaScript
2828
run: npm run lint
2929

30+
- name: Lint markdown
31+
run: npm run lint:md
3032
test:
3133
strategy:
3234
fail-fast: false

.markdownlint-cli2.cjs

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
const githubMarkdownOpinions = require('@github/markdownlint-github')
2+
3+
// Rules we want to turn on but currently have too many violations
4+
const rulesToEnforce = {
5+
'fenced-code-language': false,
6+
'no-duplicate-header': false, // Fix https://github.com/primer/doctocat/issues/527, then set this rule to `siblings_only: true`
7+
}
8+
// Rules we don't care to enforce (usually stylistic)
9+
const rulesToNotEnforce = {
10+
'line-length': false,
11+
'blanks-around-headings': false,
12+
'blanks-around-lists': false,
13+
'no-trailing-spaces': false,
14+
'no-multiple-blanks': false,
15+
'no-trailing-punctuation': false,
16+
'single-trailing-newline': false,
17+
'ul-indent': false,
18+
'no-hard-tabs': false,
19+
'first-line-heading': false,
20+
'no-space-in-emphasis': false,
21+
'blanks-around-fences': false,
22+
}
23+
24+
const defaultOverrides = {
25+
'no-generic-link-text': {exceptions: ['link']}, // We don't want it to flag links that link to `Link` component.
26+
}
27+
28+
const options = githubMarkdownOpinions.init({...rulesToNotEnforce, ...rulesToEnforce, ...defaultOverrides})
29+
module.exports = {
30+
config: options,
31+
customRules: ['@github/markdownlint-github'],
32+
outputFormatters: [
33+
['markdownlint-cli2-formatter-pretty', {appendLink: true}], // ensures the error message includes a link to the rule documentation
34+
],
35+
}

README.md

+1-5
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ yarn add @primer/react
4040

4141
## Roadmap
4242

43-
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).
43+
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).
4444

4545
## Contributing
4646

@@ -51,7 +51,3 @@ We love collaborating with folks inside and outside of GitHub and welcome contri
5151
## New Component Proposals
5252

5353
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!
54-
55-
[styled-components]: https://www.styled-components.com/docs
56-
[primer css]: https://github.com/primer/primer
57-
[flash of unstyled content]: https://en.wikipedia.org/wiki/Flash_of_unstyled_content

contributor-docs/CONTRIBUTING.md

+11-7
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ const Component = styled.div<SxProp>`
8585

8686
Component.defaultProps = {
8787
m: 0,
88-
fontSize: 5
88+
fontSize: 5,
8989
}
9090

9191
export default Component
@@ -109,6 +109,8 @@ const Component = styled.div<SxProp>`
109109

110110
### Linting
111111

112+
#### ESLint
113+
112114
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:
113115

114116
```sh
@@ -130,6 +132,14 @@ npm run lint -- --fix
130132

131133
**Protip:** `npm run lint -- --quiet` (or `npx eslint --quiet ...`) will suppress warnings so that you can focus on fixing errors.
132134

135+
#### Markdownlint
136+
137+
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:
138+
139+
```sh
140+
npm run lint:md
141+
```
142+
133143
### TypeScript support
134144

135145
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
212222

213223
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.
214224

215-
[classnames]: https://www.npmjs.com/package/classnames
216-
[spread syntax]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax
217-
[styled-system]: https://styled-system.com
218-
[table]: https://jxnblk.com/styled-system/table
219-
[npx]: https://www.npmjs.com/package/npx
220-
[now]: https://zeit.co/now
221225
[primer.style]: https://primer.style

contributor-docs/adrs/adr-004-children-as-api.md

+8-13
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,16 @@ _Note: Consumer is used multiple times on this page. It refers to the developers
1212

1313
## Decision:
1414

15-
1615
1. Prefer using children for “content”
1716

1817
2. For composite components, the API should be decided by how much customisation is available for children.
1918

2019
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.
2120

2221
```jsx
23-
<Button variant="danger" leadingIcon={TrashIcon}>Delete branch</Button>
22+
<Button variant="danger" leadingIcon={TrashIcon}>
23+
Delete branch
24+
</Button>
2425
```
2526

2627
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
3839

3940
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.
4041

41-
<img width="373" alt="image" src="https://user-images.githubusercontent.com/1863771/144945223-70c4c800-5827-4985-9f18-0ab416eba058.png">
42+
<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 -->
4243

4344
```jsx
4445
// prefer this
@@ -62,7 +63,7 @@ import {CheckIcon} from '@primer/octicons-react'
6263
render(
6364
<Flash variant="success">
6465
<CheckIcon /> Changes saved!
65-
</Flash>
66+
</Flash>,
6667
)
6768
```
6869

@@ -149,8 +150,6 @@ When intentionally going off the happy path, developers can still drop down an a
149150

150151
_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._
151152

152-
153-
154153
---
155154

156155
<br/>
@@ -233,7 +232,7 @@ We use this pattern as well in `Button`, `Button.Counter` is a restricted versio
233232

234233
<img width="184" alt="image 8" src="https://user-images.githubusercontent.com/1863771/144945218-5154b8a1-8854-4335-926c-08a4ffac6d9d.png">
235234

236-
```jsx
235+
````jsx
237236
<Button>
238237
Watch <Button.Counter>1</Button.Counter>
239238
</Button>
@@ -259,7 +258,7 @@ For Example, [legacy ActionMenu](https://primer.style/react/deprecated/ActionMen
259258
```jsx
260259
<ActionMenu overlayProps={{width: 'medium'}} anchorContent="Open column menu">
261260
</ActionMenu>
262-
```
261+
````
263262
264263
<br/>
265264
@@ -371,15 +370,13 @@ Prefer using children for “content”
371370
<Button label="Watch" variant="primary"/>
372371
```
373372
374-
375373
<img width="227" alt="image 13" src="https://user-images.githubusercontent.com/1863771/145045542-0d80491b-75e1-4304-b9fe-8c2cca80b298.png">
376374
377-
378375
The Icon should adapt to variant and size of the `Button`. We could use a `EyeIcon` in children here:
379376
380377
```jsx
381378
<Button>
382-
<EyeIcon/> Watch
379+
<EyeIcon /> Watch
383380
</Button>
384381
```
385382
@@ -393,10 +390,8 @@ But, we want to discourage customising the Icon’s color and size in the applic
393390
<Button leadingIcon={<EyeIcon/>}>Watch</Button>
394391
```
395392
396-
397393
<img width="293" alt="image 14" src="https://user-images.githubusercontent.com/1863771/145045544-1a1651f1-fbcf-4022-8e9b-b37558bb2466.png">
398394
399-
400395
We want to add a `Counter` that adapts to the variant without supporting all the props of a `CounterLabel` like `scheme`.
401396
402397
`Button.Counter` is a restricted version of `CounterLabel`, making the right thing easy and wrong thing hard:

contributor-docs/adrs/adr-005-box-sx.md

+38-38
Original file line numberDiff line numberDiff line change
@@ -10,44 +10,44 @@ In Primer React and consuming applications, we use many different patterns for c
1010

1111
1. Creating components with styled-components
1212

13-
```tsx
14-
const Avatar = styled.img.attrs<StyledAvatarProps>(props => ({
15-
height: props.size,
16-
width: props.size
17-
}))<StyledAvatarProps>`
18-
display: inline-block;
19-
overflow: hidden;
20-
line-height: ${get('lineHeights.condensedUltra')};
21-
border-radius: ${props => getBorderRadius(props)};
22-
${sx}
23-
`
24-
```
25-
26-
[Show full code example →](https://github.com/primer/react/pull/2019/files?diff=split&w=0)
27-
28-
2. Creating components with Box
29-
30-
```tsx
31-
const Avatar: React.FC<AvatarProps> = ({size = 20, alt = '', square = false, sx = {}, ...props}) => {
32-
const styles:BetterSystemStyleObject = {
33-
display: 'inline-block',
34-
overflow: 'hidden',
35-
lineHeight: 'condensedUltra',
36-
borderRadius: getBorderRadius({size, square})
37-
}
38-
39-
return (
40-
<Box
41-
as="img"
42-
alt={alt}
43-
sx={merge<BetterSystemStyleObject>(styles, sx)} // styles needs to merge with props.sx
44-
{...props}
45-
/>
46-
)
47-
}
48-
```
49-
50-
[Show full code example →](https://github.com/primer/react/pull/2019/files?diff=split&w=0)
13+
```tsx
14+
const Avatar = styled.img.attrs<StyledAvatarProps>(props => ({
15+
height: props.size,
16+
width: props.size,
17+
}))<StyledAvatarProps>`
18+
display: inline-block;
19+
overflow: hidden;
20+
line-height: ${get('lineHeights.condensedUltra')};
21+
border-radius: ${props => getBorderRadius(props)};
22+
${sx}
23+
`
24+
```
25+
26+
[Show full code example →](https://github.com/primer/react/pull/2019/files?diff=split&w=0)
27+
28+
2. Creating components with Box
29+
30+
```tsx
31+
const Avatar: React.FC<AvatarProps> = ({size = 20, alt = '', square = false, sx = {}, ...props}) => {
32+
const styles: BetterSystemStyleObject = {
33+
display: 'inline-block',
34+
overflow: 'hidden',
35+
lineHeight: 'condensedUltra',
36+
borderRadius: getBorderRadius({size, square}),
37+
}
38+
39+
return (
40+
<Box
41+
as="img"
42+
alt={alt}
43+
sx={merge<BetterSystemStyleObject>(styles, sx)} // styles needs to merge with props.sx
44+
{...props}
45+
/>
46+
)
47+
}
48+
```
49+
50+
[Show full code example →](https://github.com/primer/react/pull/2019/files?diff=split&w=0)
5151

5252
&nbsp;
5353

contributor-docs/adrs/adr-008-experimental-components.md

+4-7
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ Proposed
88

99
## Context
1010

11-
#### Recap: Drafts
11+
### Recap: Drafts
1212

1313
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.
1414

@@ -39,8 +39,6 @@ The approach that has served us well since Dec 2021 has been "drafts" (along wit
3939

4040
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.
4141

42-
43-
4442
&nbsp;
4543

4644
## Decision
@@ -53,17 +51,16 @@ The approach that has served us well since Dec 2021 has been "drafts" (along wit
5351

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

56-
#### Risks:
54+
### Risks:
5755

5856
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.
5957

60-
6158
&nbsp;
6259

6360
#### Other options considered
6461

65-
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.
66-
62+
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.
63+
6764
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.
6865

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

0 commit comments

Comments
 (0)