From 77fd8674215c0c2bd1ea447786df4f981d08a1c8 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Mon, 5 Jun 2017 19:51:31 -0700 Subject: [PATCH] [fix] correct types Fix #465 --- .github/CONTRIBUTING.md | 28 +++++--- package.json | 14 ++-- src/apis/AppRegistry/AppContainer.js | 65 +++++++++++++++++++ src/apis/AppRegistry/ReactNativeApp.js | 41 ------------ .../renderApplication-test.js.snap | 10 +++ .../__tests__/renderApplication-test.js | 6 +- src/apis/AppRegistry/index.js | 5 +- src/apis/AppRegistry/renderApplication.js | 22 ++++--- src/apis/Clipboard/index.js | 43 ++++++------ src/apis/I18nManager/index.js | 4 +- src/apis/Linking/index.js | 30 +++++---- src/apis/StyleSheet/flattenStyle.js | 2 +- src/components/ProgressBar/index.js | 14 ++-- src/components/Switch/index.js | 19 +++--- src/components/Touchable/TouchableOpacity.js | 1 + .../Touchable/TouchableWithoutFeedback.js | 2 + src/propTypes/createStrictShapeTypeChecker.js | 34 +++++----- yarn.lock | 6 +- 18 files changed, 211 insertions(+), 135 deletions(-) create mode 100644 src/apis/AppRegistry/AppContainer.js delete mode 100644 src/apis/AppRegistry/ReactNativeApp.js diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 97ea7abc..775ef63d 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -23,35 +23,47 @@ Install dependencies (requires [yarn](https://yarnpkg.com/en/docs/install): yarn ``` -## Unit tests +## Automated tests + +To run flow: + +``` +npm run flow +``` To run the unit tests: ``` -npm test +npm run jest ``` …in watch mode: ``` -npm run test:watch +npm run jest:watch +``` + +To run all automated tests: + +``` +npm test ``` ## Visual tests -Run the interactive storybook: +To run the interactive storybook: ``` npm run docs:start ``` -Run generate a static build of the storybook: +To generate a static build of the storybook: ``` npm run docs:build ``` -Run the performance benchmarks in a browser (opening `./performance/index.html`): +To run the performance benchmarks in a browser (opening `./benchmarks/index.html`): ``` npm run benchmarks @@ -59,7 +71,7 @@ npm run benchmarks ## Compile and build -Compile the source code to `dist`: +To compile the source code to `dist`: ``` npm run compile @@ -73,7 +85,7 @@ npm run build ### Pre-commit -Before creating a commit run: +To format and lint code before commit: ``` npm run precommit diff --git a/package.json b/package.json index 0192c3fd..41b4d1ed 100644 --- a/package.json +++ b/package.json @@ -18,19 +18,25 @@ "flow": "flow", "fmt": "find benchmarks docs src -name '*.js' | grep -v -E '(node_modules|dist)' | xargs npm run fmt:cmd", "fmt:cmd": "prettier --print-width=100 --single-quote --write", + "jest": "jest", + "jest:watch": "npm run test -- --watch", "lint": "npm run lint:cmd -- benchmarks docs src", "lint:cmd": "eslint --fix --ignore-path .gitignore", "precommit": "lint-staged", "release": "npm run lint && npm run test && npm run compile && npm run build && npm publish", - "test": "jest", - "test:watch": "npm run test -- --watch" + "test": "flow && jest" }, "babel": { "presets": [ "react-native" ], "plugins": [ - [ "transform-react-remove-prop-types", { "mode": "wrap" } ] + [ + "transform-react-remove-prop-types", + { + "mode": "wrap" + } + ] ] }, "jest": { @@ -77,7 +83,7 @@ "eslint-plugin-promise": "^3.5.0", "eslint-plugin-react": "^6.10.3", "file-loader": "^0.11.1", - "flow-bin": "^0.46.0", + "flow-bin": "^0.47.0", "jest": "^19.0.2", "lint-staged": "^3.4.2", "node-libs-browser": "^0.5.3", diff --git a/src/apis/AppRegistry/AppContainer.js b/src/apis/AppRegistry/AppContainer.js new file mode 100644 index 00000000..bc55ac49 --- /dev/null +++ b/src/apis/AppRegistry/AppContainer.js @@ -0,0 +1,65 @@ +/** + * @flow + */ + +import StyleSheet from '../StyleSheet'; +import View from '../../components/View'; +import { any, node } from 'prop-types'; +import React, { Component } from 'react'; + +type Context = { + rootTag: any +}; + +type Props = { + children?: React.Children, + rootTag: any +}; + +type State = { + mainKey: number +}; + +class AppContainer extends Component { + props: Props; + state: State = { mainKey: 1 }; + + static childContextTypes = { + rootTag: any + }; + + static propTypes = { + children: node, + rootTag: any.isRequired + }; + + getChildContext(): Context { + return { + rootTag: this.props.rootTag + }; + } + + render() { + return ( + + + + ); + } +} + +const styles = StyleSheet.create({ + /** + * Ensure that the application covers the whole screen. + */ + appContainer: { + flex: 1 + } +}); + +module.exports = AppContainer; diff --git a/src/apis/AppRegistry/ReactNativeApp.js b/src/apis/AppRegistry/ReactNativeApp.js deleted file mode 100644 index 765ef97b..00000000 --- a/src/apis/AppRegistry/ReactNativeApp.js +++ /dev/null @@ -1,41 +0,0 @@ -/** - * @flow - */ - -import StyleSheet from '../StyleSheet'; -import View from '../../components/View'; -import { any, object } from 'prop-types'; -import React, { Component } from 'react'; - -class ReactNativeApp extends Component { - static propTypes = { - initialProps: object, - rootComponent: any.isRequired, - rootTag: any - }; - - render() { - const { initialProps, rootComponent: RootComponent, rootTag } = this.props; - - return ( - - - - ); - } -} - -const styles = StyleSheet.create({ - /** - * Ensure that the application covers the whole screen. - */ - appContainer: { - position: 'absolute', - left: 0, - top: 0, - right: 0, - bottom: 0 - } -}); - -module.exports = ReactNativeApp; diff --git a/src/apis/AppRegistry/__tests__/__snapshots__/renderApplication-test.js.snap b/src/apis/AppRegistry/__tests__/__snapshots__/renderApplication-test.js.snap index 8ae91ad2..f1e9a9a0 100644 --- a/src/apis/AppRegistry/__tests__/__snapshots__/renderApplication-test.js.snap +++ b/src/apis/AppRegistry/__tests__/__snapshots__/renderApplication-test.js.snap @@ -1,6 +1,14 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`apis/AppRegistry/renderApplication getApplication 1`] = ` + + + +`; + +exports[`apis/AppRegistry/renderApplication getApplication 2`] = ` "" `; diff --git a/src/apis/AppRegistry/__tests__/renderApplication-test.js b/src/apis/AppRegistry/__tests__/renderApplication-test.js index 2a5f49ab..186d8512 100644 --- a/src/apis/AppRegistry/__tests__/renderApplication-test.js +++ b/src/apis/AppRegistry/__tests__/renderApplication-test.js @@ -3,13 +3,13 @@ import { getApplication } from '../renderApplication'; import React from 'react'; -const component = () =>
; +const RootComponent = () =>
; describe('apis/AppRegistry/renderApplication', () => { test('getApplication', () => { - const { element, stylesheet } = getApplication(component, {}); + const { element, stylesheet } = getApplication(RootComponent, {}); - expect(element).toBeTruthy(); + expect(element).toMatchSnapshot(); expect(stylesheet).toMatchSnapshot(); }); }); diff --git a/src/apis/AppRegistry/index.js b/src/apis/AppRegistry/index.js index 712aaa83..9ab20754 100644 --- a/src/apis/AppRegistry/index.js +++ b/src/apis/AppRegistry/index.js @@ -6,7 +6,6 @@ * @flow */ -import { Component } from 'react'; import invariant from 'fbjs/lib/invariant'; import { unmountComponentAtNode } from 'react-dom'; import renderApplication, { getApplication } from './renderApplication'; @@ -14,9 +13,9 @@ import renderApplication, { getApplication } from './renderApplication'; const emptyObject = {}; const runnables = {}; -type ComponentProvider = () => Component; +export type ComponentProvider = () => ReactClass; -type AppConfig = { +export type AppConfig = { appKey: string, component?: ComponentProvider, run?: Function diff --git a/src/apis/AppRegistry/renderApplication.js b/src/apis/AppRegistry/renderApplication.js index 9db39c21..f5f257ac 100644 --- a/src/apis/AppRegistry/renderApplication.js +++ b/src/apis/AppRegistry/renderApplication.js @@ -8,25 +8,31 @@ import invariant from 'fbjs/lib/invariant'; import { render } from 'react-dom'; -import ReactNativeApp from './ReactNativeApp'; +import AppContainer from './AppContainer'; import StyleSheet from '../../apis/StyleSheet'; -import React, { Component } from 'react'; +import React from 'react'; export default function renderApplication( - RootComponent: Component, + RootComponent: ReactClass, initialProps: Object, rootTag: any ) { invariant(rootTag, 'Expect to have a valid rootTag, instead got ', rootTag); - const component = ( - + render( + + + , + rootTag ); - render(component, rootTag); } -export function getApplication(RootComponent: Component, initialProps: Object): Object { - const element = ; +export function getApplication(RootComponent: ReactClass, initialProps: Object): Object { + const element = ( + + + + ); const stylesheet = StyleSheet.renderToString(); return { element, stylesheet }; } diff --git a/src/apis/Clipboard/index.js b/src/apis/Clipboard/index.js index 4fe4c66a..8bf0758e 100644 --- a/src/apis/Clipboard/index.js +++ b/src/apis/Clipboard/index.js @@ -16,30 +16,33 @@ class Clipboard { static setString(text) { let success = false; + const body = document.body; - // add the text to a hidden node - const node = document.createElement('span'); - node.textContent = text; - node.style.position = 'absolute'; - node.style.opacity = '0'; - document.body.appendChild(node); + if (body) { + // add the text to a hidden node + const node = document.createElement('span'); + node.textContent = text; + node.style.position = 'absolute'; + node.style.opacity = '0'; + body.appendChild(node); - // select the text - const selection = window.getSelection(); - selection.removeAllRanges(); - const range = document.createRange(); - range.selectNodeContents(node); - selection.addRange(range); + // select the text + const selection = window.getSelection(); + selection.removeAllRanges(); + const range = document.createRange(); + range.selectNodeContents(node); + selection.addRange(range); - // attempt to copy - try { - document.execCommand('copy'); - success = true; - } catch (e) {} + // attempt to copy + try { + document.execCommand('copy'); + success = true; + } catch (e) {} - // remove selection and node - selection.removeAllRanges(); - document.body.removeChild(node); + // remove selection and node + selection.removeAllRanges(); + body.removeChild(node); + } return success; } diff --git a/src/apis/I18nManager/index.js b/src/apis/I18nManager/index.js index 21529cb0..d2c66d07 100644 --- a/src/apis/I18nManager/index.js +++ b/src/apis/I18nManager/index.js @@ -24,7 +24,9 @@ const isRTL = () => { const onChange = () => { if (ExecutionEnvironment.canUseDOM) { - document.documentElement.setAttribute('dir', isRTL() ? 'rtl' : 'ltr'); + if (document.documentElement && document.documentElement.setAttribute) { + document.documentElement.setAttribute('dir', isRTL() ? 'rtl' : 'ltr'); + } } }; diff --git a/src/apis/Linking/index.js b/src/apis/Linking/index.js index 135a8739..8a4c3f82 100644 --- a/src/apis/Linking/index.js +++ b/src/apis/Linking/index.js @@ -33,19 +33,25 @@ const Linking = { */ const iframeOpen = url => { const noOpener = url.indexOf('mailto:') !== 0; - const iframe = document.createElement('iframe'); - iframe.style.display = 'none'; - document.body.appendChild(iframe); + const body = document.body; + if (body) { + const iframe = document.createElement('iframe'); + iframe.style.display = 'none'; + body.appendChild(iframe); - const iframeDoc = iframe.contentDocument || iframe.contentWindow.document; - const script = iframeDoc.createElement('script'); - const openerExpression = noOpener ? 'child.opener = null' : ''; - script.text = ` - window.parent = null; window.top = null; window.frameElement = null; - var child = window.open("${url}"); ${openerExpression}; - `; - iframeDoc.body.appendChild(script); - document.body.removeChild(iframe); + const iframeDoc = iframe.contentDocument || iframe.contentWindow.document; + const iframeBody = iframeDoc.body; + if (iframeBody) { + const script = iframeDoc.createElement('script'); + const openerExpression = noOpener ? 'child.opener = null' : ''; + script.text = ` + window.parent = null; window.top = null; window.frameElement = null; + var child = window.open("${url}"); ${openerExpression}; + `; + iframeBody.appendChild(script); + } + body.removeChild(iframe); + } }; module.exports = Linking; diff --git a/src/apis/StyleSheet/flattenStyle.js b/src/apis/StyleSheet/flattenStyle.js index 431e5613..af85be1a 100644 --- a/src/apis/StyleSheet/flattenStyle.js +++ b/src/apis/StyleSheet/flattenStyle.js @@ -23,7 +23,7 @@ function getStyle(style) { } function flattenStyle(style: ?StyleObj): ?Object { - if (!style) { + if (style == null || typeof style === 'boolean') { return undefined; } diff --git a/src/components/ProgressBar/index.js b/src/components/ProgressBar/index.js index d20d69cc..4302db61 100644 --- a/src/components/ProgressBar/index.js +++ b/src/components/ProgressBar/index.js @@ -11,6 +11,8 @@ import React, { Component } from 'react'; import { bool, number } from 'prop-types'; class ProgressBar extends Component { + _progressElement = null; + static displayName = 'ProgressBar'; static propTypes = { @@ -58,17 +60,19 @@ class ProgressBar extends Component { ); } - _setProgressRef = component => { - this._progressRef = component; + _setProgressRef = element => { + this._progressElement = element; }; _updateProgressWidth = () => { const { indeterminate, progress } = this.props; const percentageProgress = indeterminate ? 50 : progress * 100; const width = indeterminate ? '25%' : `${percentageProgress}%`; - this._progressRef.setNativeProps({ - style: { width } - }); + if (this._progressElement) { + this._progressElement.setNativeProps({ + style: { width } + }); + } }; } diff --git a/src/components/Switch/index.js b/src/components/Switch/index.js index ebaa5f4c..e0e9024d 100644 --- a/src/components/Switch/index.js +++ b/src/components/Switch/index.js @@ -18,7 +18,8 @@ const thumbDefaultBoxShadow = '0px 1px 3px rgba(0,0,0,0.5)'; const thumbFocusedBoxShadow = `${thumbDefaultBoxShadow}, 0 0 0 10px rgba(0,0,0,0.1)`; class Switch extends PureComponent { - _checkbox: HTMLInputElement; + _checkboxElement: HTMLInputElement; + _thumbElement = null; static displayName = 'Switch'; @@ -44,11 +45,11 @@ class Switch extends PureComponent { }; blur() { - UIManager.blur(this._checkbox); + UIManager.blur(this._checkboxElement); } focus() { - UIManager.focus(this._checkbox); + UIManager.focus(this._checkboxElement); } render() { @@ -136,15 +137,17 @@ class Switch extends PureComponent { _handleFocusState = (event: Object) => { const isFocused = event.nativeEvent.type === 'focus'; const boxShadow = isFocused ? thumbFocusedBoxShadow : thumbDefaultBoxShadow; - this._thumb.setNativeProps({ style: { boxShadow } }); + if (this._thumbElement) { + this._thumbElement.setNativeProps({ style: { boxShadow } }); + } }; - _setCheckboxRef = component => { - this._checkbox = component; + _setCheckboxRef = element => { + this._checkboxElement = element; }; - _setThumbRef = component => { - this._thumb = component; + _setThumbRef = element => { + this._thumbElement = element; }; } diff --git a/src/components/Touchable/TouchableOpacity.js b/src/components/Touchable/TouchableOpacity.js index c825d640..6b2ad224 100644 --- a/src/components/Touchable/TouchableOpacity.js +++ b/src/components/Touchable/TouchableOpacity.js @@ -53,6 +53,7 @@ var PRESS_RETENTION_OFFSET = { top: 20, left: 20, right: 20, bottom: 30 }; * ``` */ var TouchableOpacity = createReactClass({ + displayName: 'TouchableOpacity', mixins: [TimerMixin, Touchable.Mixin, NativeMethodsMixin], propTypes: { diff --git a/src/components/Touchable/TouchableWithoutFeedback.js b/src/components/Touchable/TouchableWithoutFeedback.js index 0df9193d..53ca4f4b 100644 --- a/src/components/Touchable/TouchableWithoutFeedback.js +++ b/src/components/Touchable/TouchableWithoutFeedback.js @@ -82,6 +82,7 @@ const TouchableWithoutFeedback = createReactClass({ * reactivated! Move it back and forth several times while the scroll view * is disabled. Ensure you pass in a constant to reduce memory allocations. */ + // $FlowFixMe pressRetentionOffset: EdgeInsetsPropType, /** * This defines how far your touch can start away from the button. This is @@ -91,6 +92,7 @@ const TouchableWithoutFeedback = createReactClass({ * of sibling views always takes precedence if a touch hits two overlapping * views. */ + // $FlowFixMe hitSlop: EdgeInsetsPropType }, diff --git a/src/propTypes/createStrictShapeTypeChecker.js b/src/propTypes/createStrictShapeTypeChecker.js index 0716ffb8..b859f0f6 100644 --- a/src/propTypes/createStrictShapeTypeChecker.js +++ b/src/propTypes/createStrictShapeTypeChecker.js @@ -1,5 +1,3 @@ -/* eslint-disable */ - /** * Copyright (c) 2015-present, Facebook, Inc. * All rights reserved. @@ -8,29 +6,28 @@ * LICENSE file in the root directory of this source tree. An additional grant * of patent rights can be found in the PATENTS file in the same directory. * + * @providesModule createStrictShapeTypeChecker * @flow */ import invariant from 'fbjs/lib/invariant'; -import ReactPropTypeLocationNames from '../vendor/ReactPropTypeLocationNames'; -import ReactPropTypesSecret from '../vendor/ReactPropTypesSecret'; function createStrictShapeTypeChecker(shapeTypes: { [key: string]: ReactPropsCheckType }): ReactPropsChainableTypeChecker { - function checkType(isRequired, props, propName, componentName, location?) { + function checkType(isRequired, props, propName, componentName, location?, ...rest) { if (!props[propName]) { if (isRequired) { invariant( false, - `Required object \`${propName}\` was not specified in ` + `\`${componentName}\`.` + `Required object \`${propName}\` was not specified in \`${componentName}\`.` ); } return; } - var propValue = props[propName]; - var propType = typeof propValue; - var locationName = (location && ReactPropTypeLocationNames[location]) || '(unknown)'; + const propValue = props[propName]; + const propType = typeof propValue; + const locationName = location || '(unknown)'; if (propType !== 'object') { invariant( false, @@ -40,24 +37,24 @@ function createStrictShapeTypeChecker(shapeTypes: { } // We need to check all keys in case some are required but missing from // props. - var allKeys = { ...props[propName], ...shapeTypes }; - for (var key in allKeys) { - var checker = shapeTypes[key]; + const allKeys = { ...props[propName], ...shapeTypes }; + for (const key in allKeys) { + const checker = shapeTypes[key]; if (!checker) { invariant( false, `Invalid props.${propName} key \`${key}\` supplied to \`${componentName}\`.` + - `\nBad object: ` + + '\nBad object: ' + JSON.stringify(props[propName], null, ' ') + - `\nValid keys: ` + + '\nValid keys: ' + JSON.stringify(Object.keys(shapeTypes), null, ' ') ); } - var error = checker(propValue, key, componentName, location, null, ReactPropTypesSecret); + const error = checker(propValue, key, componentName, location, ...rest); if (error) { invariant( false, - error.message + `\nBad object: ` + JSON.stringify(props[propName], null, ' ') + error.message + '\nBad object: ' + JSON.stringify(props[propName], null, ' ') ); } } @@ -66,9 +63,10 @@ function createStrictShapeTypeChecker(shapeTypes: { props: { [key: string]: any }, propName: string, componentName: string, - location?: string + location?: string, + ...rest ): ?Error { - return checkType(false, props, propName, componentName, location); + return checkType(false, props, propName, componentName, location, ...rest); } chainedCheckType.isRequired = checkType.bind(null, true); return chainedCheckType; diff --git a/yarn.lock b/yarn.lock index e0b2281c..6586512f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2756,9 +2756,9 @@ flatten@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/flatten/-/flatten-1.0.2.tgz#dae46a9d78fbe25292258cc1e780a41d95c03782" -flow-bin@^0.46.0: - version "0.46.0" - resolved "https://registry.yarnpkg.com/flow-bin/-/flow-bin-0.46.0.tgz#06ad7fe19dddb1042264438064a2a32fee12b872" +flow-bin@^0.47.0: + version "0.47.0" + resolved "https://registry.yarnpkg.com/flow-bin/-/flow-bin-0.47.0.tgz#a2a08ab3e0d1f1cb57d17e27b30b118b62fda367" flow-parser@0.45.0: version "0.45.0"