Repository: brigand/babel-plugin-flow-react-proptypes
Pull request #182 (2018-02-17 22:48:56)

Add support for indexed objects (`objectOf`)
Summary:
This commit adds support for types like `{[name: string]: number}`,
which describes an object with arbitrary keys and numeric values. The
corresponding PropType is `PropTypes.objectOf(PropTypes.number)`.

Two variants are rejected outright. First, Flow does not support types
with multiple indexers: `{[a: string]: number, [b: string]: number}` is
not a valid type expression. You can verify this by pasting the
following into the Flow playground:
```
type X = {[a: string]: number, [b: string]: number};
```
This gives the error:
```
1: type X = {[a: string]: number, [b: string]: number};
                                  ^ Multiple indexers are not supported.
```
It therefore seems reasonable to reject any such type expressions that
make it into the AST.

On the other hand, Flow _does_ support mixing indexers and named
properties. For instance, `{[name: string]: number, special: boolean}`
is a valid Flow type. But there’s no corresponding PropType, so we will
reject this outright.

Test Plan:
The test case `objectof-test.js` passes.

This commit caused a “regression” in `real4-test.js`, because the source
of `real4-test` actually uses indexed objects. I’ve regenerated the
snapshot to account for this.

wchargin-branch: objectof

coveralls: 
[![Coverage Status](https://coveralls.io/builds/15575233/badge)](https://coveralls.io/builds/15575233)

Coverage decreased (-0.2%) to 91.758% when pulling **8ae135ee74cf35fe64c087b467d2243a39d30c28 on wchargin:wchargin-objectof** into **393f12e9749300df82910029e6566db9d7c0eee6 on brigand:master**.


Patch:
diff --git a/src/convertToPropTypes.js b/src/convertToPropTypes.js
index f51d0ba..6477d6a 100644
--- a/src/convertToPropTypes.js
+++ b/src/convertToPropTypes.js
@@ -103,25 +103,43 @@ export default function convertToPropTypes(node, importedTypes, internalTypes) {
   if (node.type === 'ObjectTypeAnnotation') {
     const properties = [];
 
-    // recurse on object properties
-    node.properties.forEach((subnode) => {
-      // result may be from:
-      //  ObjectTypeProperty - {key, value}
-      //  ObjectTypeSpreadProperty - Array<{key, value}>
-      const result = convertToPropTypes(subnode, importedTypes, internalTypes);
-      if (subnode.leadingComments && subnode.leadingComments.length) {
-        result.leadingComments = subnode.leadingComments;
+    if (node.indexers.length > 0) {
+      // indexed object, like `{[name: string]: number}`
+      if (node.indexers.length > 1) {
+        throw new Error(`${PLUGIN_NAME}: Multiple indexers are not supported. Node: ${JSON.stringify(node)}`);
       }
-      if (Array.isArray(result)){
-        result.forEach((prop) => properties.push(prop));
+      if (node.properties.length > 0) {
+        throw new Error(`${PLUGIN_NAME}: Mixed indexers and named properties are not supported. Node: ${JSON.stringify(node)}`);
       }
-      else {
-        properties.push(result);
-      }
-    });
+      // recur on indexer's value type (`number` in the example above)
+      // and emit an `objectOf`
+      const indexer = node.indexers[0];
+      const subnode = indexer.value;
+      const subresult = convertToPropTypes(subnode, importedTypes, internalTypes);
+      resultPropType = {type: 'objectOf', of: subresult};
+    }
+    else {
+      // not an indexed object
+      // recurse on object properties
+      node.properties.forEach((subnode) => {
+        // result may be from:
+        //  ObjectTypeProperty - {key, value}
+        //  ObjectTypeSpreadProperty - Array<{key, value}>
+        const result = convertToPropTypes(subnode, importedTypes, internalTypes);
+        if (subnode.leadingComments && subnode.leadingComments.length) {
+          result.leadingComments = subnode.leadingComments;
+        }
+        if (Array.isArray(result)){
+          result.forEach((prop) => properties.push(prop));
+        }
+        else {
+          properties.push(result);
+        }
+      });
 
-    // return a shape
-    resultPropType = {type: 'shape', properties, isExact: node.exact};
+      // return a shape
+      resultPropType = {type: 'shape', properties, isExact: node.exact};
+    }
   }
   else if (node.type === 'ObjectTypeProperty') {
     const key = getObjectTypePropertyKey(node);
diff --git a/src/makePropTypesAst.js b/src/makePropTypesAst.js
index 4a4d2b4..d53afe9 100644
--- a/src/makePropTypesAst.js
+++ b/src/makePropTypesAst.js
@@ -347,6 +347,12 @@ function makePropType(data, isExact) {
       [makePropType(data.of)]
     );
   }
+  else if (method === 'objectOf') {
+    node = t.callExpression(
+      t.memberExpression(node, t.identifier('objectOf')),
+      [makePropType(data.of)]
+    );
+  }
   else if (method === 'oneOf') {
     node = t.callExpression(
       t.memberExpression(node, t.identifier('oneOf')),


Test patch:
diff --git a/src/__tests__/__snapshots__/objectof-test.js.snap b/src/__tests__/__snapshots__/objectof-test.js.snap
new file mode 100644
index 0000000..9b70a1c
--- /dev/null
+++ b/src/__tests__/__snapshots__/objectof-test.js.snap
@@ -0,0 +1,49 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`objectOf 1`] = `
+"'use strict';
+
+Object.defineProperty(exports, \\"__esModule\\", {
+  value: true
+});
+
+var _propTypes = require('prop-types');
+
+var _propTypes2 = _interopRequireDefault(_propTypes);
+
+function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }
+
+function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError(\\"Cannot call a class as a function\\"); } }
+
+function _possibleConstructorReturn(self, call) { if (!self) { throw new ReferenceError(\\"this hasn't been initialised - super() hasn't been called\\"); } return call && (typeof call === \\"object\\" || typeof call === \\"function\\") ? call : self; }
+
+function _inherits(subClass, superClass) { if (typeof superClass !== \\"function\\" && superClass !== null) { throw new TypeError(\\"Super expression must either be null or a function, not \\" + typeof superClass); } subClass.prototype = Object.create(superClass && superClass.prototype, { constructor: { value: subClass, enumerable: false, writable: true, configurable: true } }); if (superClass) Object.setPrototypeOf ? Object.setPrototypeOf(subClass, superClass) : subClass.__proto__ = superClass; }
+
+var React = require('react');
+
+var Foo = function (_React$Component) {
+  _inherits(Foo, _React$Component);
+
+  function Foo() {
+    _classCallCheck(this, Foo);
+
+    return _possibleConstructorReturn(this, (Foo.__proto__ || Object.getPrototypeOf(Foo)).apply(this, arguments));
+  }
+
+  return Foo;
+}(React.Component);
+
+Foo.propTypes = {
+  oneNumber: _propTypes2.default.number.isRequired,
+  oneComplex: _propTypes2.default.shape({
+    real: _propTypes2.default.number.isRequired,
+    imag: _propTypes2.default.number.isRequired
+  }).isRequired,
+  manyNumbers: _propTypes2.default.objectOf(_propTypes2.default.number).isRequired,
+  manyComplex: _propTypes2.default.objectOf(_propTypes2.default.shape({
+    real: _propTypes2.default.number.isRequired,
+    imag: _propTypes2.default.number.isRequired
+  })).isRequired
+};
+exports.default = Foo;"
+`;
diff --git a/src/__tests__/__snapshots__/real4-test.js.snap b/src/__tests__/__snapshots__/real4-test.js.snap
index 51c8e5b..b713292 100644
--- a/src/__tests__/__snapshots__/real4-test.js.snap
+++ b/src/__tests__/__snapshots__/real4-test.js.snap
@@ -42,7 +42,11 @@ var bpfrpt_proptype_searchPostResult = {
     total: _propTypes2.default.number.isRequired,
     pages: _propTypes2.default.number.isRequired,
     resultId: _propTypes2.default.string.isRequired,
-    facets: _propTypes2.default.shape({})
+    facets: _propTypes2.default.objectOf(_propTypes2.default.arrayOf(_propTypes2.default.shape({
+        key: _propTypes2.default.number.isRequired,
+        amount: _propTypes2.default.number.isRequired,
+        translation: _propTypes2.default.string.isRequired
+    })))
 };
 
 // eslint-disable-next-line
diff --git a/src/__tests__/objectof-mixed-indexers-named-test.js b/src/__tests__/objectof-mixed-indexers-named-test.js
new file mode 100644
index 0000000..c8e715a
--- /dev/null
+++ b/src/__tests__/objectof-mixed-indexers-named-test.js
@@ -0,0 +1,20 @@
+const babel = require('babel-core');
+const content = `
+var React = require('react');
+
+type FooProps = {
+  problem: {[name: string]: number, special: string},
+}
+
+export default class Foo extends React.Component {
+  props: FooProps
+}
+`;
+
+it('objectOf rejects multiple indexers', () => {
+  expect(() => babel.transform(content, {
+    babelrc: false,
+    presets: ['es2015', 'stage-1', 'react'],
+    plugins: ['syntax-flow', require('../')],
+  })).toThrow(/mixed indexers and named properties are not supported/i);
+});
diff --git a/src/__tests__/objectof-multiple-indexers-test.js b/src/__tests__/objectof-multiple-indexers-test.js
new file mode 100644
index 0000000..e421eff
--- /dev/null
+++ b/src/__tests__/objectof-multiple-indexers-test.js
@@ -0,0 +1,20 @@
+const babel = require('babel-core');
+const content = `
+var React = require('react');
+
+type FooProps = {
+  problem: {[name: string]: number, [nombre: string]: bool},
+}
+
+export default class Foo extends React.Component {
+  props: FooProps
+}
+`;
+
+it('objectOf rejects multiple indexers', () => {
+  expect(() => babel.transform(content, {
+    babelrc: false,
+    presets: ['es2015', 'stage-1', 'react'],
+    plugins: ['syntax-flow', require('../')],
+  })).toThrow(/multiple indexers are not supported/i);
+});
diff --git a/src/__tests__/objectof-test.js b/src/__tests__/objectof-test.js
new file mode 100644
index 0000000..fb2adb4
--- /dev/null
+++ b/src/__tests__/objectof-test.js
@@ -0,0 +1,28 @@
+const babel = require('babel-core');
+const content = `
+var React = require('react');
+
+type Complex = {
+  real: number;
+  imag: number;
+}
+type FooProps = {
+  oneNumber: number;
+  oneComplex: Complex;
+  manyNumbers: {[name: string]: number},
+  manyComplex: {[name: string]: Complex},
+}
+
+export default class Foo extends React.Component {
+  props: FooProps
+}
+`;
+
+it('objectOf', () => {
+  const res = babel.transform(content, {
+    babelrc: false,
+    presets: ['es2015', 'stage-1', 'react'],
+    plugins: ['syntax-flow', require('../')],
+  }).code;
+  expect(res).toMatchSnapshot();
+});