Repository: brigand/babel-plugin-flow-react-proptypes
Pull request #88 (2017-05-27 06:20:41)

Fix functional component props
Another one. See:

https://github.com/brigand/babel-plugin-flow-react-proptypes/pull/88/commits/f2b1e09f8f2c97bb68c9f5a1b554a876a5c85109

This MR shows all commits of #87 because I pushed before you merged that one, but it really is only the one above + a merge commit.

brigand: Could you rebase this on master?
brigand: It looks like this is setting C.propTypes to a function; it needs to be an object.
mhaas: Rebased.
mhaas: @brigand Should be good now. I am omitting the non-useful cases now.

Patch:
diff --git a/src/index.js b/src/index.js
index 27f4875..5dae5a0 100644
--- a/src/index.js
+++ b/src/index.js
@@ -6,7 +6,7 @@ import {
   hasReactElementTypeAnnotationReturn,
 } from './util';
 import convertToPropTypes from './convertToPropTypes';
-import makePropTypesAst from './makePropTypesAst';
+import {makePropTypesAstForExport, makePropTypesAstForPropTypesAssignment} from './makePropTypesAst';
 
 // maps between type alias name to prop types
 let internalTypes = {};
@@ -28,15 +28,9 @@ const convertNodeToPropTypes = node => convertToPropTypes(
 );
 
 const getPropsForTypeAnnotation = typeAnnotation => {
-  const typeAnnotationReference = typeAnnotation.id && typeAnnotation.id.name;
   let props = null;
-  if (typeAnnotationReference) {
-    props = internalTypes[typeAnnotationReference] || importedTypes[typeAnnotationReference];
-    if (!props) {
-      $debug(`Did not find type annotation for reference ${typeAnnotationReference}`);
-    }
-  }
-  else if (typeAnnotation.properties || typeAnnotation.type === 'GenericTypeAnnotation'
+
+  if (typeAnnotation.properties || typeAnnotation.type === 'GenericTypeAnnotation'
       || typeAnnotation.type === 'IntersectionTypeAnnotation'
       || typeAnnotation.type === 'AnyTypeAnnotation') {
     props = convertNodeToPropTypes(typeAnnotation);
@@ -109,18 +103,10 @@ module.exports = function flowReactPropTypes(babel) {
       throw new Error(`Did not find type annotation for ${name}`);
     }
 
-
-    if (!props.properties) {
-      // Bail out if we don't have any properties. This will be the case if
-      // we have an imported PropType, like:
-      // import type { T } from '../types';
-      // const C = (props: T) => <div>{props.name}</div>;
-
-      // TODO: this case is still handled elsewhere, correct?
+    const propTypesAST = makePropTypesAstForPropTypesAssignment(props);
+    if (propTypesAST == null) {
       return;
     }
-
-    const propTypesAST = makePropTypesAst(props);
     const attachPropTypesAST = t.expressionStatement(
       t.assignmentExpression(
         '=',
@@ -256,7 +242,7 @@ module.exports = function flowReactPropTypes(babel) {
         const propTypes = convertNodeToPropTypes(declarationObject);
         internalTypes[name] = propTypes;
 
-        const propTypesAst = makePropTypesAst(propTypes);
+        const propTypesAst = makePropTypesAstForExport(propTypes);
 
         const exportAst = t.expressionStatement(t.callExpression(
           t.memberExpression(t.identifier('Object'), t.identifier('defineProperty')),
diff --git a/src/makePropTypesAst.js b/src/makePropTypesAst.js
index 62fd9ea..c3b4913 100644
--- a/src/makePropTypesAst.js
+++ b/src/makePropTypesAst.js
@@ -4,19 +4,19 @@ import template from 'babel-template';
 
 const USE_PROPTYPES_PACKAGE = true;
 
-
 /**
  * Top-level function to generate prop-types AST.
  *
- * This will return an expression sufficient for assignment to the
- * propTypes property of an React component.
- *
- * For complex types, this will be an object, not a shape.
+ * This will return an expression suitable for assignment to the
+ * propTypes property of an React component. In particular,
+ * the AST returned will always yield an object. Assignment
+ * to Foo.propTypes is possible for objects, not simple
+ * types.
  *
  * @param propTypeData Intermediate representation
- * @returns {*} AST
+ * @returns {*} AST expression always returning an object.
  */
-export default function makePropTypesAst(propTypeData) {
+export function makePropTypesAstForPropTypesAssignment(propTypeData) {
   if (propTypeData.type === 'shape-intersect-runtime') {
     // For top-level usage, e.g. Foo.proptype, return
     // an expression returning an object.
@@ -25,13 +25,47 @@ export default function makePropTypesAst(propTypeData) {
   else if (propTypeData.type === 'shape') {
     return makeObjectAstForShape(propTypeData);
   }
-  else {
-    // Required to handle non-object exports.
-    return makePropType(propTypeData);
+  else if (propTypeData.type === 'raw') {
+    return makeObjectAstForRaw(propTypeData);
+  }
+};
+
+
+/**
+ * Top-level function to generate prop-types AST.
+ *
+ * This will return an expression suitable for exporting.
+ *
+ * This function is similar to makePropTypesAstForPropTypesAssignment, except
+ * that you may export non-object expressions.
+ *
+ * Any items not handled by makePropTypesAstForPropTypesAssignment will be returned
+ * as an AST invoking the corresponding function from the prop-types package.
+ *
+ * @param propTypeData Intermediate representation
+ * @returns {*} AST for expression resulting in object or function
+ */
+export function makePropTypesAstForExport(propTypeData) {
+  let ast = makePropTypesAstForPropTypesAssignment(propTypeData);
+  if (ast == null) {
+    ast = makePropType(propTypeData);
   }
+  return ast;
 };
 
 
+function makeObjectAstForRaw(propTypeSpec, propTypeObjects) {
+  let propTypeObject = t.identifier(propTypeSpec.value);
+
+  // This will just be a variable, referencing an import we
+  // generated above. This variable may contain prop-types.any,
+  // which will not work when used in an intersection.
+  const importNode = makePropTypeImportNode();
+  const anyNode = t.memberExpression(importNode, t.identifier('any'));
+  const testExpression = t.binaryExpression('===', propTypeObject, anyNode);
+  propTypeObject = t.conditionalExpression(testExpression, t.objectExpression([]), propTypeObject);
+  return propTypeObject;
+}
 /**
  * Generates AST for run-time merges involving either import variables,
  * local types (shape) and other run-time merges.
@@ -69,16 +103,7 @@ function makeObjectMergeAstForShapeIntersectRuntime(propTypeData) {
   const propTypeObjects = [];
   propTypeData.properties.forEach(propTypeSpec => {
     if (propTypeSpec.type === 'raw') {
-      let propTypeObject = t.identifier(propTypeSpec.value);
-
-      // This will just be a variable, referencing an import we
-      // generated above. This variable may contain prop-types.any,
-      // which will not work when used in an intersection.
-      const importNode = makePropTypeImportNode();
-      const anyNode = t.memberExpression(importNode, t.identifier('any'));
-      const testExpression = t.binaryExpression('===', propTypeObject, anyNode);
-      propTypeObject = t.conditionalExpression(testExpression, t.objectExpression([]), propTypeObject);
-      propTypeObjects.push(propTypeObject);
+      propTypeObjects.push(makeObjectAstForRaw(propTypeSpec));
 
     }
     else if (propTypeSpec.type === 'shape') {


Test patch:
diff --git a/src/__tests__/__snapshots__/stateless-inline-imports-test.js.snap b/src/__tests__/__snapshots__/stateless-inline-imports-test.js.snap
index 43a4a3f..1b772d2 100644
--- a/src/__tests__/__snapshots__/stateless-inline-imports-test.js.snap
+++ b/src/__tests__/__snapshots__/stateless-inline-imports-test.js.snap
@@ -15,5 +15,6 @@ var C = function C(props) {
     null,
     props.name
   );
-};"
+};
+C.propTypes = babelPluginFlowReactPropTypes_proptype_T === require(\'prop-types\').any ? {} : babelPluginFlowReactPropTypes_proptype_T;"
 `;
diff --git a/src/__tests__/__snapshots__/top-level-import-test.js.snap b/src/__tests__/__snapshots__/top-level-import-test.js.snap
index 2f20e05..a6b03ed 100644
--- a/src/__tests__/__snapshots__/top-level-import-test.js.snap
+++ b/src/__tests__/__snapshots__/top-level-import-test.js.snap
@@ -46,5 +46,6 @@ var Test = function (_Component) {
   return Test;
 }(_react.Component);
 
+Test.propTypes = babelPluginFlowReactPropTypes_proptype_Data === require(\'prop-types\').any ? {} : babelPluginFlowReactPropTypes_proptype_Data;
 exports.default = Test;"
 `;