Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

TypeScript refactor #959

Merged
merged 29 commits into from
Jan 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
b6458df
Install @babel/preset-typescript
colebemis Jan 9, 2021
23c8eaf
index.js → index.ts
colebemis Jan 9, 2021
666f5c3
Create tsconfig.json
colebemis Jan 9, 2021
2fc81bb
constants.js → constants.ts
colebemis Jan 9, 2021
68715a3
sx.js → sx.ts
colebemis Jan 9, 2021
699ce1c
Box.js → Box.tsx
colebemis Jan 9, 2021
2531d4a
Export BoxProps interface
colebemis Jan 9, 2021
f67eb88
Add "dist:types" script
colebemis Jan 9, 2021
52cce3f
Clean up "dist" scripts
colebemis Jan 9, 2021
35fb829
Use @babel/eslint-parser
colebemis Jan 9, 2021
af86a1c
Install gatsby-plugin-typescript
colebemis Jan 12, 2021
ea4f79c
Install typescript in docs
colebemis Jan 12, 2021
912c737
Update webpack js rule
colebemis Jan 12, 2021
7e30bf6
Filter out tsx rule
colebemis Jan 12, 2021
b816dda
Try removing rule overrides
colebemis Jan 12, 2021
79481b9
Remove playroom from now build
colebemis Jan 12, 2021
28de9c6
Import box types
colebemis Jan 12, 2021
0b0de5e
Create StyledComponentProps interface
colebemis Jan 12, 2021
3898ce0
Add html attributes to box props
colebemis Jan 12, 2021
919b46f
Use TypeScript for Box tests
colebemis Jan 12, 2021
09354a3
Refactor BoxProps
colebemis Jan 13, 2021
e24f64b
Restore rule overrides
colebemis Jan 13, 2021
04dd454
Remove typescript dep from docs
colebemis Jan 13, 2021
cf7b3ce
Rename system prop interfaces
colebemis Jan 14, 2021
c2ca680
Explain @ts-ignore reason
colebemis Jan 14, 2021
6f93b2e
Move @types/styled-system__* deps
colebemis Jan 14, 2021
f0508b1
Merge remote-tracking branch 'origin/main' into colebemis/ts-refactor
colebemis Jan 14, 2021
decbbf3
Revert index.d.ts
colebemis Jan 15, 2021
083129e
Remove types directory from published files
colebemis Jan 15, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"parser": "@babel/eslint-parser",
"extends": [
"plugin:github/es6",
"plugin:github/react",
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ lib/
lib-esm/
public/
stats.html
types
2 changes: 1 addition & 1 deletion babel.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ function replacementPlugin(env) {
const sharedPlugins = ['macros', 'preval', 'add-react-displayname', 'babel-plugin-styled-components', '@babel/plugin-proposal-nullish-coalescing-operator']

function makePresets(moduleValue) {
return [['@babel/preset-react', {modules: moduleValue}]]
return ['@babel/preset-typescript', ['@babel/preset-react', {modules: moduleValue}]]
}

module.exports = {
Expand Down
1 change: 1 addition & 0 deletions docs/gatsby-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module.exports = {
description: 'React components for the Primer design system'
},
plugins: [
'gatsby-plugin-typescript',
{
resolve: '@primer/gatsby-theme-doctocat',
options: {
Expand Down
1 change: 1 addition & 0 deletions docs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"@styled-system/theme-get": "^5.1.0",
"gatsby": "2.27.5",
"gatsby-plugin-alias-imports": "^1.0.5",
"gatsby-plugin-typescript": "2.10.0",
"prop-types": "^15.7.2",
"react": "^17.0.0",
"react-dom": "^17.0.0",
Expand Down
18 changes: 18 additions & 0 deletions docs/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3085,6 +3085,11 @@ babel-plugin-remove-graphql-queries@^2.11.0:
resolved "https://registry.yarnpkg.com/babel-plugin-remove-graphql-queries/-/babel-plugin-remove-graphql-queries-2.11.0.tgz#0d90f7c3e66033eebccf69b5c2618f1239e89ff0"
integrity sha512-JJiC1JdMubakyCAppYnFqdXqX1sNRIYomoRQ1tH2xzXHrsmhk3lRUSozsEGWxsL7r9D7zacuhi6fF4vukIyo0w==

babel-plugin-remove-graphql-queries@^2.14.0:
version "2.14.0"
resolved "https://registry.yarnpkg.com/babel-plugin-remove-graphql-queries/-/babel-plugin-remove-graphql-queries-2.14.0.tgz#d34dbc8aaa4eb2b6f11165c63bf3c226b283194b"
integrity sha512-7uc3CzyMZRuGVU69RZ/vJ4+jYeuyIbT9dTMjX2wu3QJ+TcRAaW3MJVxG5cN7YhCBv0S0/KIqXdh6BpSYswzUkw==

"babel-plugin-styled-components@>= 1":
version "1.12.0"
resolved "https://registry.yarnpkg.com/babel-plugin-styled-components/-/babel-plugin-styled-components-1.12.0.tgz#1dec1676512177de6b827211e9eda5a30db4f9b9"
Expand Down Expand Up @@ -6653,6 +6658,19 @@ gatsby-plugin-styled-components@^3.1.0:
dependencies:
"@babel/runtime" "^7.12.5"

gatsby-plugin-typescript@2.10.0:
version "2.10.0"
resolved "https://registry.yarnpkg.com/gatsby-plugin-typescript/-/gatsby-plugin-typescript-2.10.0.tgz#d6cd190384804eb8b7d1e1fb1e6e132b8fa0bdda"
integrity sha512-1mf7zrFVE5UEut+3YRnb3N2XareM0Xk4Mfzw0K89ASqWT4R4QaEIxycsjPERBZMFy0GLWt8e0Hl93Wwn+eefwg==
dependencies:
"@babel/core" "^7.12.3"
"@babel/plugin-proposal-nullish-coalescing-operator" "^7.12.1"
"@babel/plugin-proposal-numeric-separator" "^7.12.5"
"@babel/plugin-proposal-optional-chaining" "^7.12.1"
"@babel/preset-typescript" "^7.12.1"
"@babel/runtime" "^7.12.5"
babel-plugin-remove-graphql-queries "^2.14.0"

gatsby-plugin-typescript@^2.7.0:
version "2.7.0"
resolved "https://registry.yarnpkg.com/gatsby-plugin-typescript/-/gatsby-plugin-typescript-2.7.0.tgz#a21820a6e115f919a04229c526cc76a622c86cf4"
Expand Down
3 changes: 1 addition & 2 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,7 @@ declare module '@primer/components' {
Content: React.FunctionComponent<PopoverContentProps>
}

export interface PositionComponentProps
extends PositionProps, BoxProps {}
export interface PositionComponentProps extends PositionProps, BoxProps {}

export const Relative: React.FunctionComponent<PositionComponentProps>
export const Absolute: React.FunctionComponent<PositionComponentProps>
Expand Down
6 changes: 0 additions & 6 deletions now.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"version": 2,
"alias": "primer-components.now.sh",
"routes": [
{"src": "/playroom(/.*)?", "dest": "$1"},
{"src": "/components(/.*)?", "dest": "/docs$1"},
{
"src": "/",
Expand All @@ -16,11 +15,6 @@
"src": "docs/package.json",
"use": "@now/static-build",
"config": {"distDir": "public"}
},
{
"src": "package.json",
"use": "@now/static-build",
"config": {"distDir": "public"}
}
]
}
20 changes: 13 additions & 7 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
"typings": "index.d.ts",
"scripts": {
"start": "cd docs && yarn run develop",
"predist": "rm -rf dist",
"dist": "yarn dist:rollup && yarn dist:transpile:cjs && yarn dist:transpile:esm",
"dist:rollup": "cross-env NODE_ENV=production rollup -c",
"dist:transpile:cjs": "rm -rf lib && cross-env BABEL_MODULE=commonjs babel src --out-dir lib",
"dist:transpile:esm": "rm -rf lib-esm && babel src --out-dir lib-esm",
"dist": "concurrently yarn:dist:*",
"dist:rollup": "rm -rf dist && cross-env NODE_ENV=production rollup -c",
"dist:transpile:cjs": "rm -rf lib && cross-env BABEL_MODULE=commonjs babel src --out-dir lib --extensions '.js,.ts,.jsx,.tsx'",
"dist:transpile:esm": "rm -rf lib-esm && babel src --out-dir lib-esm --extensions '.js,.ts,.jsx,.tsx'",
"dist:types": "rm -rf types && tsc",
"prepublishOnly": "yarn run dist",
"lint": "eslint src docs/components",
"test": "jest -- src",
Expand Down Expand Up @@ -47,21 +47,25 @@
"@styled-system/props": "5.1.4",
"@styled-system/theme-get": "5.1.2",
"@types/styled-system": "5.1.2",
"@types/styled-system__css": "5.0.14",
"@types/styled-system__theme-get": "5.0.1",
"@types/styled-components": "^4.4.0",
"classnames": "^2.2.5",
"polished": "3.5.2",
"react-is": "16.10.2",
"styled-system": "5.1.2"
},
"devDependencies": {
"@testing-library/react": "9.4.0",
"@babel/cli": "7.8.4",
"@babel/core": "7.9.0",
"@babel/eslint-parser": "7.12.1",
"@babel/plugin-proposal-nullish-coalescing-operator": "7.12.1",
"@babel/plugin-transform-modules-commonjs": "7.12.1",
"@babel/preset-react": "7.9.4",
"@babel/preset-typescript": "7.12.7",
"@rollup/plugin-commonjs": "16.0.0",
"@rollup/plugin-node-resolve": "7.1.3",
"@types/jest": "26.0.20",
"@testing-library/dom": "7.29.0",
"@testing-library/react": "11.2.2",
"@testing-library/user-event": "12.6.0",
Expand All @@ -73,6 +77,7 @@
"babel-plugin-styled-components": "1.10.7",
"babel-plugin-transform-replace-expressions": "0.2.0",
"babel-polyfill": "6.26.0",
"concurrently": "5.3.0",
"cross-env": "7.0.2",
"enzyme": "3.10.0",
"eslint": "6.5.1",
Expand All @@ -93,7 +98,8 @@
"rollup-plugin-terser": "5.3.0",
"rollup-plugin-visualizer": "4.0.4",
"semver": "7.3.2",
"styled-components": "4.4.0"
"styled-components": "4.4.0",
"typescript": "4.1.3"
},
"peerDependencies": {
"react": "^17.0.0",
Expand Down
9 changes: 6 additions & 3 deletions rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,21 @@ import resolve from '@rollup/plugin-node-resolve'
import {terser} from 'rollup-plugin-terser'
import visualizer from 'rollup-plugin-visualizer'

const extensions = ['.js', '.jsx', '.ts', '.tsx']

const formats = ['esm', 'umd']

const plugins = [
babel({exclude: 'node_modules/**', runtimeHelpers: true}),
resolve(),
babel({extensions, exclude: 'node_modules/**', runtimeHelpers: true}),
resolve({extensions}),
commonjs(),
terser(),
visualizer({sourcemap: true})
]

export default [
{
input: 'src/index.js',
input: 'src/index.ts',
external: ['styled-components', 'react', 'react-dom'],
plugins,
output: formats.map(format => ({
Expand Down
9 changes: 5 additions & 4 deletions src/Box.js → src/Box.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import styled from 'styled-components'
import PropTypes from 'prop-types'
import sx from './sx'
import {COMMON, FLEX, LAYOUT} from './constants'
import styled from 'styled-components'
import {COMMON, SystemCommonProps, FLEX, SystemFlexProps, LAYOUT, SystemLayoutProps} from './constants'
import sx, {SxProp} from './sx'
import theme from './theme'

const Box = styled.div`
const Box = styled.div<SystemCommonProps & SystemFlexProps & SystemLayoutProps & SxProp>`
${COMMON}
${FLEX}
${LAYOUT}
Expand All @@ -21,4 +21,5 @@ Box.propTypes = {
theme: PropTypes.object,
}

export type BoxProps = React.ComponentProps<typeof Box>
export default Box
Comment on lines +24 to 25
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth re-considering using default exports as well? It is probably a breaking change if anyone was consuming directly from @primer/components/lib/Box or something like that, but if we're going to be making other breaking changes, it might be a fair time to consider it.

I'm not really aware of the main pros/cons of default exports, and it seems like in other code at GitHub we're inconsistent on this already, so maybe it's not even worth looking at 😅

For example, https://github.com/github/details-dialog-element/blob/main/src/index.ts uses a default export, and web components in the dotcom codebase itself seem to be about 50/50.

Just thought I'd mention it to see if it sparks thoughts for others.

Copy link
Contributor

Choose a reason for hiding this comment

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

Generally I prefer not to use default exports. Here is a good article about it: https://basarat.gitbook.io/typescript/main-1/defaultisbad

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm open to moving away from default exports for our components. A few thoughts:

  • If we decide to remove default exports, I'd prefer to do that in a separate PR that updates all the exports of all the components at once. This would allow consumers to update all their imports at once.
  • How do we ensure consistency across the codebase? Is there an existing linting rule for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

10 changes: 5 additions & 5 deletions src/__tests__/Box.js → src/__tests__/Box.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import {cleanup, render as HTMLRender} from '@testing-library/react'
import 'babel-polyfill'
import {axe, toHaveNoViolations} from 'jest-axe'
import React from 'react'
import {Box} from '..'
import {COMMON, FLEX, LAYOUT} from '../constants'
import theme from '../theme'
import {render, behavesAsComponent, checkExports} from '../utils/testing'
import {LAYOUT, COMMON, FLEX} from '../constants'
import {render as HTMLRender, cleanup} from '@testing-library/react'
import {axe, toHaveNoViolations} from 'jest-axe'
import 'babel-polyfill'
import {behavesAsComponent, checkExports, render} from '../utils/testing'
expect.extend(toHaveNoViolations)

describe('Box', () => {
Expand Down
47 changes: 0 additions & 47 deletions src/constants.js

This file was deleted.

92 changes: 92 additions & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// @ts-ignore @styled-system/prop-types does not provide type definitions
import systemPropTypes from '@styled-system/prop-types'
import {themeGet} from '@styled-system/theme-get'
import PropTypes from 'prop-types'
import React from 'react'
import * as styledSystem from 'styled-system'
import theme from './theme'

interface StyleFn extends styledSystem.styleFn {
propTypes?: React.WeakValidationMap<any>
}

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

export const get = (key: string) => themeGet(key, getKey(theme, key))

// Common props

export const COMMON: StyleFn = compose(styledSystem.space, styledSystem.color, styledSystem.display)

export interface SystemCommonProps
extends styledSystem.ColorProps,
styledSystem.SpaceProps,
styledSystem.DisplayProps {}

COMMON.propTypes = {
...systemPropTypes.space,
...systemPropTypes.color,
}

// Typography props

const whiteSpace = system({
whiteSpace: {
property: 'whiteSpace',
// cssProperty: 'whiteSpace',
},
})

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

export interface SystemTypographyProps extends styledSystem.TypographyProps {
whiteSpace?: 'normal' | 'nowrap' | 'pre' | 'pre-wrap' | 'pre-line'
}

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

// Border props

export const BORDER: StyleFn = compose(styledSystem.border, styledSystem.shadow)

export interface SystemBorderProps extends styledSystem.BorderProps, styledSystem.ShadowProps {}

BORDER.propTypes = {
...systemPropTypes.border,
...systemPropTypes.shadow,
}

// Layout props

export const LAYOUT: StyleFn = styledSystem.layout

export interface SystemLayoutProps extends styledSystem.LayoutProps {}

LAYOUT.propTypes = systemPropTypes.layout

// Position props

export const POSITION: StyleFn = styledSystem.position

export interface SystemPositionProps extends styledSystem.PositionProps {}

POSITION.propTypes = systemPropTypes.position

// Flex props

export const FLEX: StyleFn = styledSystem.flexbox

export interface SystemFlexProps extends styledSystem.FlexboxProps {}

FLEX.propTypes = systemPropTypes.flexbox

// Grid props

export const GRID: StyleFn = styledSystem.grid

export interface SystemGridProps extends styledSystem.GridProps {}

GRID.propTypes = systemPropTypes.grid
File renamed without changes.
10 changes: 0 additions & 10 deletions src/sx.js

This file was deleted.

14 changes: 14 additions & 0 deletions src/sx.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import PropTypes from 'prop-types'
import css, {SystemStyleObject} from '@styled-system/css'

export interface SxProp {
sx?: SystemStyleObject
}

const sx = (props: SxProp) => css(props.sx)

sx.propTypes = {
sx: PropTypes.object,
}

export default sx
Loading