Skip to content

Commit

Permalink
Merge pull request #1068 from primer/cb/remove-prop-types
Browse files Browse the repository at this point in the history
Remove propTypes
  • Loading branch information
colebemis authored Mar 1, 2021
2 parents 4e81af6 + 2dfc6f2 commit d738e99
Show file tree
Hide file tree
Showing 75 changed files with 209 additions and 1,041 deletions.
5 changes: 5 additions & 0 deletions .changeset/polite-geese-smile.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/components": major
---

Remove propTypes in favor of TypeScript types
132 changes: 50 additions & 82 deletions contributor-docs/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,28 @@

1. [Roadmap](#roadmap)
2. [Before Getting Started](#before-getting-started)
2. [Discussing non-public features or products](#discussing-non-public-features-or-products)
2. [Developing Components](#developing-components)
* [Tools we use](#tools-we-use)
* [Component patterns](#component-patterns)
* [Adding default theme](#adding-default-theme)
* [Adding system props](#adding-system-props)
* [Adding the sx prop](#adding-the-sx-prop)
* [Linting](#linting)
* [Testing](#testing)
* [TypeScript support](#typescript-support)
* [Additional resources](#additional-resources)
3. [Writing documentation](#writing-documentation)
4. [Creating a pull request](#creating-a-pull-request)
* [What to expect after opening a pull request](#what-to-expect-after-opening-a-pull-request)
* [What we look for in reviews](#what-we-look-for-in-reviews)
5. [Deploying & publishing](#deploying-and-publishing)
* [Deploying](#deploying)
* [Path aliasing](#path-aliasing)
* [Publishing](#publishing)
6. [Troubleshooting](#troubleshooting)
7. [Glossary](#glossary)
* [System Props](#system-props)
3. [Discussing non-public features or products](#discussing-non-public-features-or-products)
4. [Developing Components](#developing-components)
- [Tools we use](#tools-we-use)
- [Component patterns](#component-patterns)
- [Adding default theme](#adding-default-theme)
- [Adding system props](#adding-system-props)
- [Adding the sx prop](#adding-the-sx-prop)
- [Linting](#linting)
- [Testing](#testing)
- [TypeScript support](#typescript-support)
- [Additional resources](#additional-resources)
5. [Writing documentation](#writing-documentation)
6. [Creating a pull request](#creating-a-pull-request)
- [What to expect after opening a pull request](#what-to-expect-after-opening-a-pull-request)
- [What we look for in reviews](#what-we-look-for-in-reviews)
7. [Deploying & publishing](#deploying-and-publishing)
- [Deploying](#deploying)
- [Path aliasing](#path-aliasing)
- [Publishing](#publishing)
8. [Troubleshooting](#troubleshooting)
9. [Glossary](#glossary)
- [System Props](#system-props)

## Roadmap

Expand All @@ -43,7 +43,6 @@ A common question asked about Primer Components is how to know what should be ad

As this is a public repo, please be careful not to include details or screenshots from unreleased GitHub products or features. In most cases, a good bug report, feature request, or pull request should be able to describe the work without including business logic or feature details, but if you must discuss context relating to an unreleased feature, please open an issue in the private [Design Systems repo](https://github.com/github/design-systems/issues/new/choose) and link to it in your issue or pull request.


## Developing components

We primarily use our documentation site as a workspace to develop new components or make changes to existing components (stay tuned for a better development environment coming soon!).
Expand Down Expand Up @@ -73,11 +72,9 @@ With a couple of exceptions, all components should be created with the `styled`

Default values for system props can be set in `Component.defaultProps`.

Prop Types from system props such as `COMMON` or `TYPOGRAPHY` as well as styled-system functions can be spread in the component's prop types declaration (see example below). These need to go *after* any built-in styles that you want to be overridable.

⚠️ **Make sure to always set the default `theme` prop to our [theme](https://github.com/primer/components/blob/main/src/theme-preval.js)! This allows consumers of our components to access our theme values without a ThemeProvider.**
⚠️ **Make sure to always set the default `theme` prop to our [theme](https://github.com/primer/components/blob/main/src/theme-preval.js)! This allows consumers of our components to access our theme values without a ThemeProvider.**

Additionally, every component should support [the `sx` prop](https://primer.style/components/overriding-styles); remember to add `${sx}` to the style literal and `...sx.propTypes` to the component's `propTypes`.
Additionally, every component should support [the `sx` prop](https://primer.style/components/overriding-styles); remember to add `${sx}` to the style literal.

Here's an example of a basic component written in the style of Primer Components:

Expand All @@ -100,12 +97,6 @@ Component.defaultProps = {
fontSize: 5,
}
Component.propTypes = {
...COMMON.propTypes,
...TYPOGRAPHY.propTypes,
...sx.propTypes
}
export default Component
```
Expand All @@ -119,9 +110,8 @@ To add the default theme to a component, import the theme and assign it to the c
import theme from './theme'
Component.defaultProps = {
theme, // make sure to always set the default theme!
theme // make sure to always set the default theme!
}
```

### Adding system props
Expand All @@ -130,12 +120,12 @@ Each component should have access to the appropriate system props. Every compone
Categories of system props are exported from `src/constants.js`:
* `COMMON` includes color and spacing (margin and padding) props
* `TYPOGRAPHY` includes font family, font weight, and line-height props
* `POSITION` includes positioning props
* `FLEX` includes flexbox props
* `BORDER` includes border and box-shadow props
* `GRID` includes grid props
- `COMMON` includes color and spacing (margin and padding) props
- `TYPOGRAPHY` includes font family, font weight, and line-height props
- `POSITION` includes positioning props
- `FLEX` includes flexbox props
- `BORDER` includes border and box-shadow props
- `GRID` includes grid props
To give the component access to a group of system props, import the system prop function from `./constants` and include the system prop function in your styled-component like so:
Expand All @@ -146,21 +136,15 @@ const Component = styled.div`
// additional styles here
${COMMON};
`
// don't forget to also add it to your prop type declaration!

Component.propTypes = {
...COMMON.propTypes
}
```
Remember that the system prop function inside your style declaration needs to go *after* any built-in styles you want to be overridable.
Remember that the system prop function inside your style declaration needs to go _after_ any built-in styles you want to be overridable.
### Adding the `sx` prop
Each component should provide access to a prop called `sx` that allows for setting theme-aware ad-hoc styles. See the [overriding styles](https://primer.style/components/overriding-styles) doc for more information on using the prop.
Adding the `sx` prop is similar to adding system props; import the default export from the `sx` module, add it to your style definition, and add the appropriate prop types. **The `sx` prop should go at the *very end* of your style definition.**
Adding the `sx` prop is similar to adding system props; import the default export from the `sx` module, add it to your style definition, and add the appropriate prop types. **The `sx` prop should go at the _very end_ of your style definition.**
```jsx
import {COMMON} from './constants'
Expand All @@ -171,13 +155,6 @@ const Component = styled.div`
${COMMON};
${sx};
`

// don't forget to also add it to your prop type declaration!

Component.propTypes = {
...COMMON.propTypes,
...sx.propTypes
}
```
### Linting
Expand Down Expand Up @@ -221,11 +198,11 @@ Whenever adding new components or modifying the props of an existing component,
### Additional resources
* [Primer Components Philosophy](https://primer.style/components/philosophy)
* [Primer Components Core Concepts](https://primer.style/components/core-concepts)
* [Primer Components System Props](https://primer.style/components/system-props)
* [Styled Components docs](https://styled-components.com/)
* [Styled System docs](https://styled-system.com/)
- [Primer Components Philosophy](https://primer.style/components/philosophy)
- [Primer Components Core Concepts](https://primer.style/components/core-concepts)
- [Primer Components System Props](https://primer.style/components/system-props)
- [Styled Components docs](https://styled-components.com/)
- [Styled System docs](https://styled-system.com/)
## Writing documentation
Expand All @@ -243,16 +220,16 @@ After opening a pull request, a member of the design systems team will add the a

### What we look for in reviews

* If it's a new component, does the component make sense to add to Primer Components? (Ideally this is discussed before the pull request stage, please reach out to a DS member if you aren't sure if a component should be added to Primer Components!)
* Does the component follow our [Primer Components code style](#component-patterns)?
* Does the component use theme values for most CSS values?
* Does the component have access to the [default theme](#adding-default-theme)?
* Does the component have the [correct system props implemented](#adding-system-props)?
* Is the component API intuitive?
* Does the component have the appropriate [type definitions in `index.d.ts`](#typescript-support)?
* Is the component documented accurately?
* Does the component have appropriate tests?
* Does the pull request increase the bundle size significantly?
- If it's a new component, does the component make sense to add to Primer Components? (Ideally this is discussed before the pull request stage, please reach out to a DS member if you aren't sure if a component should be added to Primer Components!)
- Does the component follow our [Primer Components code style](#component-patterns)?
- Does the component use theme values for most CSS values?
- Does the component have access to the [default theme](#adding-default-theme)?
- Does the component have the [correct system props implemented](#adding-system-props)?
- Is the component API intuitive?
- Does the component have the appropriate [type definitions in `index.d.ts`](#typescript-support)?
- Is the component documented accurately?
- Does the component have appropriate tests?
- Does the pull request increase the bundle size significantly?

If everything looks great, the design systems team member will approve the pull request and merge when appropriate. Minor and patch changes are released frequently, and we try to bundle up breaking changes and avoid shipping major versions too often. If your pull request is time-sensitive, please let a design systems team member know. You do not need to worry about merging pull requests on your own, we'll take care of that for you :)
Expand Down Expand Up @@ -286,7 +263,7 @@ We use [changesets](https://github.com/atlassian/changesets) to managing version
**`yarn start` fails with an error like `gatsby: command not found`**
Make sure to run `yarn` from inside the `docs/` subfolder *as well as* the root folder.
Make sure to run `yarn` from inside the `docs/` subfolder _as well as_ the root folder.
** `yarn start` fails with a different error**
Expand All @@ -306,19 +283,10 @@ const SpaceDiv = styled.div`
`
```
System props come with `propTypes` that can be mixed into your own with ES6 [spread syntax]:

```jsx
SpaceDiv.propTypes = {
stellar: PropTypes.bool,
...space.propTypes
}
```

[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
[now]: https://zeit.co/now
[primer.style]: https://primer.style
5 changes: 0 additions & 5 deletions docs/components/PropsList.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import React from 'react'
import PropTypes from 'prop-types'

const PropsList = ({systemProps}) => <div>{systemProps.propNames.join(', ')}</div>

PropsList.propTypes = {
systemProps: PropTypes.arrayOf(PropTypes.string)
}

export default PropsList
19 changes: 3 additions & 16 deletions docs/components/constants.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import * as styledSystem from 'styled-system'
import PropTypes from 'prop-types'
import {theme} from '@primer/components'
import systemPropTypes from '@styled-system/prop-types'
import themeGet from '@styled-system/theme-get'
import {theme} from '@primer/components'
import * as styledSystem from 'styled-system'

const {get: getKey, compose, system} = styledSystem

Expand All @@ -17,21 +16,9 @@ const whiteSpace = system({

export const TYPOGRAPHY = compose(styledSystem.typography, whiteSpace)

TYPOGRAPHY.propTypes = {
...systemPropTypes.typography,
whiteSpace: PropTypes.oneOf(['normal', 'nowrap', 'pre-wrap', 'pre', 'pre-line'])
}

export const COMMON = compose(styledSystem.space, styledSystem.color, styledSystem.display)
COMMON.propTypes = {
...systemPropTypes.space,
...systemPropTypes.color
}

export const BORDER = compose(styledSystem.border, styledSystem.shadow)
BORDER.propTypes = {
...systemPropTypes.border,
...systemPropTypes.shadow
}

// these are 1:1 with styled-system's API now,
// so you could consider dropping the abstraction
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
"@primer/octicons-react": "^11.3.0",
"@primer/primitives": "0.0.0-20211167520",
"@styled-system/css": "5.1.5",
"@styled-system/prop-types": "5.1.2",
"@styled-system/props": "5.1.4",
"@styled-system/theme-get": "5.1.2",
"@types/history": "4.7.8",
Expand Down
9 changes: 0 additions & 9 deletions src/Avatar.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import PropTypes from 'prop-types'
import styled from 'styled-components'
import {COMMON, get, SystemCommonProps} from './constants'
import sx, {SxProp} from './sx'
Expand Down Expand Up @@ -39,13 +38,5 @@ Avatar.defaultProps = {
square: false
}

Avatar.propTypes = {
...COMMON.propTypes,
size: PropTypes.number,
square: PropTypes.bool,
...sx.propTypes,
theme: PropTypes.object
}

export type AvatarProps = ComponentProps<typeof Avatar>
export default Avatar
5 changes: 0 additions & 5 deletions src/AvatarPair.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import PropTypes from 'prop-types'
import React from 'react'
import styled from 'styled-components'
import Avatar from './Avatar'
Expand Down Expand Up @@ -31,9 +30,5 @@ const AvatarPair = ({children, ...rest}: AvatarPairProps) => {
AvatarPair.displayName = 'AvatarPair'

AvatarPair.defaultProps = {theme}
AvatarPair.propTypes = {
...Relative.propTypes,
theme: PropTypes.object
}

export default AvatarPair
13 changes: 3 additions & 10 deletions src/AvatarStack.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import React from 'react'
import classnames from 'classnames'
import PropTypes from 'prop-types'
import React from 'react'
import styled from 'styled-components'
import {COMMON, get, SystemCommonProps} from './constants'
import {Absolute} from './Position'
import sx, {SxProp} from './sx'
import {get, COMMON, SystemCommonProps} from './constants'
import theme from './theme'
import {Absolute} from './Position'
import {ComponentProps} from './utils/types'

type StyledAvatarStackWrapperProps = {
Expand Down Expand Up @@ -164,10 +163,4 @@ AvatarStack.defaultProps = {
theme
}

AvatarStack.propTypes = {
...COMMON.propTypes,
alignRight: PropTypes.bool,
...sx.propTypes
}

export default AvatarStack
7 changes: 0 additions & 7 deletions src/BaseStyles.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import PropTypes from 'prop-types'
import React from 'react'
import styled, {createGlobalStyle} from 'styled-components'
import {COMMON, SystemCommonProps, SystemTypographyProps, TYPOGRAPHY} from './constants'
Expand Down Expand Up @@ -52,10 +51,4 @@ BaseStyles.defaultProps = {
theme
}

BaseStyles.propTypes = {
...TYPOGRAPHY.propTypes,
...COMMON.propTypes,
theme: PropTypes.object
}

export default BaseStyles
8 changes: 0 additions & 8 deletions src/BorderBox.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import PropTypes from 'prop-types'
import styled from 'styled-components'
import Box from './Box'
import {BORDER, SystemBorderProps} from './constants'
Expand All @@ -19,12 +18,5 @@ BorderBox.defaultProps = {
borderRadius: 2
}

BorderBox.propTypes = {
...Box.propTypes,
...BORDER.propTypes,
...sx.propTypes,
theme: PropTypes.object
}

export type BorderBoxProps = ComponentProps<typeof BorderBox>
export default BorderBox
9 changes: 0 additions & 9 deletions src/Box.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import PropTypes from 'prop-types'
import styled from 'styled-components'
import {COMMON, FLEX, LAYOUT, SystemCommonProps, SystemFlexProps, SystemLayoutProps} from './constants'
import sx, {SxProp} from './sx'
Expand All @@ -14,13 +13,5 @@ const Box = styled.div<SystemCommonProps & SystemFlexProps & SystemLayoutProps &

Box.defaultProps = {theme}

Box.propTypes = {
...COMMON.propTypes,
...FLEX.propTypes,
...LAYOUT.propTypes,
...sx.propTypes,
theme: PropTypes.object
}

export type BoxProps = ComponentProps<typeof Box>
export default Box
Loading

1 comment on commit d738e99

@vercel
Copy link

@vercel vercel bot commented on d738e99 Mar 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.