From 5744d3c8600f7ac546a6041f9328b90b92d11531 Mon Sep 17 00:00:00 2001 From: William Chargin Date: Sat, 17 Feb 2018 13:30:16 -0800 Subject: [PATCH] Autogenerate PropTypes from Flow types (#20) Summary: Closes #17; see discussion there. This commit uses the `babel-plugin-flow-react-proptypes` package to automatically create PropType definitions from components that are typed with Flow. It simultaneously updates all of our existing components to be typed with Flow. As a result, we have both static and dynamic type checking. Test Plan: Note that `yarn test` and `yarn flow` report no errors, and that there are no prop validation errors at runtime with `yarn start`. Then, apply the following patch: ```diff diff --git a/explorer/src/UserExplorer.js b/explorer/src/UserExplorer.js index bb574cd..636a10d 100644 --- a/explorer/src/UserExplorer.js +++ b/explorer/src/UserExplorer.js @@ -18,7 +18,7 @@ export class UserExplorer extends Component<{ .sort((a,b) => b[1] - a[1]); const entries = sortedUserWeightTuples.map(authorWeight => { const [author, weight] = authorWeight; - return + return }); return

User Explorer

``` Note that `yarn test` fails (the `App.test.js` E2E rendering test), `yarn flow` fails, and there is a runtime prop validation error. wchargin-branch: autogenerate-proptypes --- explorer/package.json | 6 ++++++ explorer/src/App.js | 4 +++- explorer/src/FileExplorer.js | 40 ++++++++++++++++-------------------- explorer/src/UserExplorer.js | 33 ++++++++++++++--------------- explorer/src/commitUtils.js | 22 ++------------------ explorer/yarn.lock | 17 +++++++++++---- 6 files changed, 57 insertions(+), 65 deletions(-) diff --git a/explorer/package.json b/explorer/package.json index c3ce6ff..be3eb8c 100644 --- a/explorer/package.json +++ b/explorer/package.json @@ -85,11 +85,17 @@ ] }, "babel": { + "plugins": [ + "flow-react-proptypes" + ], "presets": [ "react-app" ] }, "eslintConfig": { "extends": "react-app" + }, + "devDependencies": { + "babel-plugin-flow-react-proptypes": "^17.1.2" } } diff --git a/explorer/src/App.js b/explorer/src/App.js index 51d1081..19c182f 100644 --- a/explorer/src/App.js +++ b/explorer/src/App.js @@ -1,10 +1,12 @@ +// @flow import React, { Component } from 'react'; import data from './data.json'; import './App.css'; import { FileExplorer } from './FileExplorer.js'; import { UserExplorer } from './UserExplorer.js'; -class App extends Component { +type AppState = {selectedPath: string, selectedUser: ?string}; +class App extends Component<{}, AppState> { constructor() { super(); this.state = { diff --git a/explorer/src/FileExplorer.js b/explorer/src/FileExplorer.js index 41f11cc..34a715b 100644 --- a/explorer/src/FileExplorer.js +++ b/explorer/src/FileExplorer.js @@ -1,15 +1,13 @@ +// @flow import React, { Component } from 'react'; -import PropTypes from 'prop-types'; import {buildTree} from './commitUtils'; -import {propTypes as commitUtilsPropTypes} from './commitUtils'; - -export class FileExplorer extends Component { - static propTypes = { - selectedPath: PropTypes.string, - onSelectPath: PropTypes.func.isRequired, - data: commitUtilsPropTypes.commitData.isRequired, - } +import type {CommitData, FileTree} from './commitUtils'; +export class FileExplorer extends Component<{ + selectedPath: string, + onSelectPath: (newPath: string) => void, + data: CommitData, +}> { render() { // within the FileExplorer, paths start with "./", outside they don't // which is hacky and should be cleaned up @@ -38,19 +36,16 @@ export class FileExplorer extends Component { } } -class FileEntry extends Component { - static propTypes = { - name: PropTypes.string.isRequired, - path: PropTypes.string.isRequired, - alwaysExpand: PropTypes.bool.isRequired, - - // The type for the tree is recursive, and is annoying to specify as - // a proptype. The Flow type definition is in commitUtils.js. - tree: PropTypes.object.isRequired, - - selectedPath: PropTypes.string.isRequired, - onSelectPath: PropTypes.func.isRequired, - } +class FileEntry extends Component<{ + name: string, + path: string, + alwaysExpand: bool, + tree: FileTree, + selectedPath: string, + onSelectPath: (newPath: string) => void, +}, { + expanded: bool, +}> { constructor() { super(); @@ -89,4 +84,5 @@ class FileEntry extends Component { {(this.state.expanded || this.props.alwaysExpand) && subEntries}
} + } diff --git a/explorer/src/UserExplorer.js b/explorer/src/UserExplorer.js index 8da252d..bb574cd 100644 --- a/explorer/src/UserExplorer.js +++ b/explorer/src/UserExplorer.js @@ -1,18 +1,14 @@ +// @flow import React, { Component } from 'react'; -import PropTypes from 'prop-types'; -import { - commitWeight, - propTypes as commitUtilsPropTypes, - userWeightForPath, -} from './commitUtils'; +import {commitWeight, userWeightForPath} from './commitUtils'; +import type {CommitData, FileTree} from './commitUtils'; -export class UserExplorer extends Component { - static propTypes = { - selectedPath: PropTypes.string.isRequired, - selectedUser: PropTypes.string, - onSelectUser: PropTypes.func.isRequired, - data: commitUtilsPropTypes.commitData.isRequired, - } +export class UserExplorer extends Component<{ + selectedPath: string, + selectedUser: ?string, + onSelectUser: (newUser: string) => void, + data: CommitData, +}> { render() { const weights = userWeightForPath(this.props.selectedPath, this.props.data, commitWeight); @@ -29,16 +25,16 @@ export class UserExplorer extends Component { {entries} } + } /** * Record the cred earned by the user in a given scope. */ -class UserEntry extends Component { - static propTypes = { - userId: PropTypes.string.isRequired, - weight: PropTypes.number.isRequired, - } +class UserEntry extends Component<{ + userId: string, + weight: number, +}> { render() { return
@@ -46,4 +42,5 @@ class UserEntry extends Component { {this.props.weight.toFixed(1)}
} + } diff --git a/explorer/src/commitUtils.js b/explorer/src/commitUtils.js index 4cca4a0..870ec0d 100644 --- a/explorer/src/commitUtils.js +++ b/explorer/src/commitUtils.js @@ -2,30 +2,12 @@ import PropTypes from 'prop-types'; -type CommitData = { +export type CommitData = { // TODO improve variable names fileToCommits: {[filename: string]: string[]}; commits: {[commithash: string]: Commit}; - authors: string[]; } -export const propTypes = { - commitData: PropTypes.shape({ - fileToCommits: PropTypes.objectOf( - PropTypes.arrayOf(PropTypes.string.isRequired).isRequired, - ).isRequired, - commits: PropTypes.objectOf(PropTypes.shape({ - author: PropTypes.string.isRequired, - stats: PropTypes.objectOf(PropTypes.shape({ - lines: PropTypes.number.isRequired, - insertions: PropTypes.number.isRequired, - deletions: PropTypes.number.isRequired, - }).isRequired).isRequired, - }).isRequired).isRequired, - }), -}; - - type Commit = { author: string; stats: {[filename: string]: FileStats}; @@ -86,7 +68,7 @@ export function userWeightForPath(path: string, data: CommitData, weightFn: Weig return userWeightMap; } -type FileTree = {[string]: FileTree}; +export type FileTree = {[string]: FileTree}; export function buildTree(fileNames: string[]): FileTree { const sortedFileNames = fileNames.slice().sort(); diff --git a/explorer/yarn.lock b/explorer/yarn.lock index 019bb7e..bc75ae0 100644 --- a/explorer/yarn.lock +++ b/explorer/yarn.lock @@ -323,7 +323,7 @@ babel-code-frame@6.26.0, babel-code-frame@^6.11.0, babel-code-frame@^6.22.0, bab esutils "^2.0.2" js-tokens "^3.0.2" -babel-core@6.26.0, babel-core@^6.0.0, babel-core@^6.26.0: +babel-core@6.26.0, babel-core@^6.0.0, babel-core@^6.25.0, babel-core@^6.26.0: version "6.26.0" resolved "https://registry.yarnpkg.com/babel-core/-/babel-core-6.26.0.tgz#af32f78b31a6fcef119c87b0fd8d9753f03a0bb8" dependencies: @@ -514,6 +514,15 @@ babel-plugin-dynamic-import-node@1.1.0: babel-template "^6.26.0" babel-types "^6.26.0" +babel-plugin-flow-react-proptypes@^17.1.2: + version "17.1.2" + resolved "https://registry.yarnpkg.com/babel-plugin-flow-react-proptypes/-/babel-plugin-flow-react-proptypes-17.1.2.tgz#89f75928a47ea869dab312605f42542dd7b6755c" + dependencies: + babel-core "^6.25.0" + babel-template "^6.25.0" + babel-traverse "^6.25.0" + babel-types "^6.25.0" + babel-plugin-istanbul@^4.0.0: version "4.1.5" resolved "https://registry.yarnpkg.com/babel-plugin-istanbul/-/babel-plugin-istanbul-4.1.5.tgz#6760cdd977f411d3e175bb064f2bc327d99b2b6e" @@ -913,7 +922,7 @@ babel-runtime@6.26.0, babel-runtime@^6.18.0, babel-runtime@^6.22.0, babel-runtim core-js "^2.4.0" regenerator-runtime "^0.11.0" -babel-template@^6.16.0, babel-template@^6.24.1, babel-template@^6.26.0: +babel-template@^6.16.0, babel-template@^6.24.1, babel-template@^6.25.0, babel-template@^6.26.0: version "6.26.0" resolved "https://registry.yarnpkg.com/babel-template/-/babel-template-6.26.0.tgz#de03e2d16396b069f46dd9fff8521fb1a0e35e02" dependencies: @@ -923,7 +932,7 @@ babel-template@^6.16.0, babel-template@^6.24.1, babel-template@^6.26.0: babylon "^6.18.0" lodash "^4.17.4" -babel-traverse@^6.18.0, babel-traverse@^6.23.1, babel-traverse@^6.24.1, babel-traverse@^6.26.0: +babel-traverse@^6.18.0, babel-traverse@^6.23.1, babel-traverse@^6.24.1, babel-traverse@^6.25.0, babel-traverse@^6.26.0: version "6.26.0" resolved "https://registry.yarnpkg.com/babel-traverse/-/babel-traverse-6.26.0.tgz#46a9cbd7edcc62c8e5c064e2d2d8d0f4035766ee" dependencies: @@ -937,7 +946,7 @@ babel-traverse@^6.18.0, babel-traverse@^6.23.1, babel-traverse@^6.24.1, babel-tr invariant "^2.2.2" lodash "^4.17.4" -babel-types@^6.18.0, babel-types@^6.19.0, babel-types@^6.23.0, babel-types@^6.24.1, babel-types@^6.26.0: +babel-types@^6.18.0, babel-types@^6.19.0, babel-types@^6.23.0, babel-types@^6.24.1, babel-types@^6.25.0, babel-types@^6.26.0: version "6.26.0" resolved "https://registry.yarnpkg.com/babel-types/-/babel-types-6.26.0.tgz#a3b073f94ab49eb6fa55cd65227a334380632497" dependencies: