Skip to content

Commit

Permalink
Fix issue #66 (#99)
Browse files Browse the repository at this point in the history
* Fix issue #66

Any function containing a react component is considered a functional
React component by isFunctionalReactComponent().

This leads to false positives for functions which just use or generate
instances of React components. These factory functions do not
follow the general contract of react components. In particular, the
first argument to the function may not be props.

It is hard to detect these cases in isFunctionalReactComponent. Instead,
we relax the sanity check in getPropsForTypeAnnotation: if a typeAnnotation
exists but does not have the type we expect, we do not fail hard.
We just don't generate proptypes.

* Add missing changelog for 3.1.2

* Add changelog entry for #66
  • Loading branch information
mhaas authored and brigand committed May 31, 2017
1 parent 0bce70b commit 65c2402
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 0 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## NEXT
- Fix error on attempted propTypes generation for non-component function (#66)

## 3.1.2
- Fix bug with functions defaulting to react components (#97)

## 3.1.0

- Add support for top-level propTypes assignment of imported types (#88)
Expand Down
14 changes: 14 additions & 0 deletions src/__tests__/__snapshots__/test-issue-66.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
exports[`test issue 66 1`] = `
"\"use strict\";
Object.defineProperty(exports, \"__esModule\", {
value: true
});
exports.default = function (url, options) {
var Html = arguments.length > 2 && arguments[2] !== undefined ? arguments[2] : DefaultHtml;
return React.createElement(\"div\", null);
};"
`;
25 changes: 25 additions & 0 deletions src/__tests__/test-issue-66.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
const babel = require('babel-core');
const content = `
// @flow
export default function(
url: string,
options: PhenomicStaticConfig,
Html: Function = DefaultHtml
): Promise<string> {
return <div/>;
}
`;

it('issue 66', () => {
const res = babel.transform(content, {
babelrc: false,
presets: ['es2015', 'stage-1', 'react'],
plugins: ['syntax-flow', require('../')],
}).code;
expect(res).toMatchSnapshot();
});
3 changes: 3 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ const getPropsForTypeAnnotation = typeAnnotation => {
|| typeAnnotation.type === 'AnyTypeAnnotation') {
props = convertNodeToPropTypes(typeAnnotation);
}
else if (typeAnnotation.properties != null || typeAnnotation.type != null) {
$debug('typeAnnotation not of expected type, not generating propTypes: ', typeAnnotation);
}
else {
throw new Error(`Expected prop types, but found none. This is a bug in ${PLUGIN_NAME}`);
}
Expand Down

0 comments on commit 65c2402

Please sign in to comment.