Skip to content

Commit b15e1d4

Browse files
authored
Fix intersection and union flow types (#118)
* Refactor for flow intersection and union Props Refactor to resolve, when possible, a list of props from a flow intersection type and to throw an error when encountering a Props of union type. This change addresses a bug found in the flowTypeDocBlockHandler while a props type that is an IntersectionTypeAnnotation or a UnionTypeAnnotation. Union types throw an error, rather than generating misleading documentation. When given a union type, flow will enforce that props conform to only one of the union-ed types. The documentation format, which lists a single set of props can't express this. Fix lint errors * Enable no-undef and no-unused-variables eslint rules * Do not throw if union type encountered, but silently ignore
1 parent 4f6c5ee commit b15e1d4

11 files changed

+122
-42
lines changed

.eslintrc

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,13 @@
44
"comma-dangle": [1, "always-multiline"],
55
"no-underscore-dangle": 0,
66
"quotes": [2, "single", "avoid-escape"],
7-
"strict": 0
7+
"strict": 0,
8+
"no-unused-vars": 2,
9+
"no-undef": 2
810
},
911
"env": {
10-
"node": true
12+
"node": true,
13+
"es6": true
1114
},
1215
"globals": {
1316
"ASTNode": true,

flow/react-docgen.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ type FlowTypeDescriptor = {
4141
elements?: Array<FlowTypeDescriptor>,
4242
type?: 'object' | 'function',
4343
signature?: flowObjectSignatureType | flowFunctionSignatureType,
44+
alias?: string,
4445
};
4546

4647
type PropDescriptor = {

src/handlers/__tests__/flowTypeDocBlockHandler-test.js

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,52 @@ describe('flowTypeDocBlockHandler', () => {
127127
.not.toThrow();
128128
});
129129

130+
it('supports intersection proptypes', () => {
131+
var definition = statement(`
132+
(props: Props) => <div />;
133+
134+
var React = require('React');
135+
import type Imported from 'something';
136+
137+
type BarProps = {
138+
/** bar description */
139+
bar: 'barValue'
140+
}
141+
142+
type Props = Imported & BarProps & {
143+
/** foo description */
144+
foo: 'fooValue'
145+
};
146+
`).get('expression');
147+
148+
flowTypeDocBlockHandler(documentation, definition);
149+
150+
expect(documentation.descriptors).toEqual({
151+
foo: {
152+
description: 'foo description',
153+
},
154+
bar: {
155+
description: 'bar description',
156+
},
157+
});
158+
159+
it('does not support union proptypes', () => {
160+
var definition = statement(`
161+
(props: Props) => <div />;
162+
163+
var React = require('React');
164+
import type Imported from 'something';
165+
166+
type Other = { bar: 'barValue' };
167+
type Props = Imported | Other | { foo: 'fooValue' };
168+
`).get('expression');
169+
170+
expect(() => flowTypeDocBlockHandler(documentation, definition))
171+
.toThrowError(TypeError, "react-docgen doesn't support Props of union types");
172+
});
173+
});
174+
175+
130176
describe('does not error for unreachable type', () => {
131177
function test(code) {
132178
var definition = statement(code).get('expression');

src/handlers/__tests__/flowTypeHandler-test.js

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -170,14 +170,13 @@ describe('flowTypeHandler', () => {
170170
it('supports intersection proptypes', () => {
171171
var definition = statement(`
172172
(props: Props) => <div />;
173-
173+
174174
var React = require('React');
175175
import type Imported from 'something';
176-
176+
177177
type Props = Imported & { foo: 'bar' };
178178
`).get('expression');
179179

180-
181180
flowTypeHandler(documentation, definition);
182181

183182
expect(documentation.descriptors).toEqual({
@@ -188,24 +187,21 @@ describe('flowTypeHandler', () => {
188187
});
189188
});
190189

191-
it('supports union proptypes', () => {
190+
it('does not support union proptypes', () => {
192191
var definition = statement(`
193192
(props: Props) => <div />;
194-
193+
195194
var React = require('React');
196195
import type Imported from 'something';
197196
198-
type Props = Imported | { foo: 'bar' };
197+
type Other = { bar: 'barValue' };
198+
type Props = Imported | Other | { foo: 'fooValue' };
199199
`).get('expression');
200200

201-
flowTypeHandler(documentation, definition);
201+
expect(() => flowTypeHandler(documentation, definition))
202+
.not.toThrow();
202203

203-
expect(documentation.descriptors).toEqual({
204-
foo: {
205-
flowType: {},
206-
required: true,
207-
},
208-
});
204+
expect(documentation.descriptors).toEqual({});
209205
});
210206

211207
describe('does not error for unreachable type', () => {

src/handlers/componentMethodsHandler.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import getMemberValuePath from '../utils/getMemberValuePath';
1515
import getMethodDocumentation from '../utils/getMethodDocumentation';
1616
import isReactComponentClass from '../utils/isReactComponentClass';
1717
import isReactComponentMethod from '../utils/isReactComponentMethod';
18-
import isReactCreateClassCall from '../utils/isReactCreateClassCall';
1918

2019
import type Documentation from '../Documentation';
2120

src/handlers/flowTypeDocBlockHandler.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,23 @@
1212

1313
import type Documentation from '../Documentation';
1414
import setPropDescription from '../utils/setPropDescription';
15-
import getFlowTypeFromReactComponent from '../utils/getFlowTypeFromReactComponent';
15+
import getFlowTypeFromReactComponent, {
16+
applyToFlowTypeProperties,
17+
} from '../utils/getFlowTypeFromReactComponent';
1618

1719
/**
1820
* This handler tries to find flow Type annotated react components and extract
1921
* its types to the documentation. It also extracts docblock comments which are
2022
* inlined in the type definition.
2123
*/
2224
export default function flowTypeDocBlockHandler(documentation: Documentation, path: NodePath) {
23-
let flowTypesPath = getFlowTypeFromReactComponent(path);
25+
const flowTypesPath = getFlowTypeFromReactComponent(path);
2426

2527
if (!flowTypesPath) {
2628
return;
2729
}
2830

29-
flowTypesPath.get('properties').each(propertyPath => setPropDescription(documentation, propertyPath));
31+
applyToFlowTypeProperties(flowTypesPath, propertyPath =>
32+
setPropDescription(documentation, propertyPath)
33+
);
3034
}

src/handlers/flowTypeHandler.js

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,30 +14,19 @@ import type Documentation from '../Documentation';
1414

1515
import getFlowType from '../utils/getFlowType';
1616
import getPropertyName from '../utils/getPropertyName';
17-
import getFlowTypeFromReactComponent from '../utils/getFlowTypeFromReactComponent';
17+
import getFlowTypeFromReactComponent, {
18+
applyToFlowTypeProperties,
19+
} from '../utils/getFlowTypeFromReactComponent';
1820

1921
function setPropDescriptor(documentation: Documentation, path: NodePath): void {
2022
const propDescriptor = documentation.getPropDescriptor(getPropertyName(path));
2123
const type = getFlowType(path.get('value'));
22-
2324
if (type) {
2425
propDescriptor.flowType = type;
2526
propDescriptor.required = !path.node.optional;
2627
}
2728
}
2829

29-
function findAndSetTypes(documentation: Documentation, path: NodePath): void {
30-
if (path.node.properties) {
31-
path.get('properties').each(
32-
propertyPath => setPropDescriptor(documentation, propertyPath)
33-
);
34-
} else if (path.node.types) {
35-
path.get('types').each(
36-
typesPath => findAndSetTypes(documentation, typesPath)
37-
);
38-
}
39-
}
40-
4130
/**
4231
* This handler tries to find flow Type annotated react components and extract
4332
* its types to the documentation. It also extracts docblock comments which are
@@ -50,5 +39,7 @@ export default function flowTypeHandler(documentation: Documentation, path: Node
5039
return;
5140
}
5241

53-
findAndSetTypes(documentation, flowTypesPath);
42+
applyToFlowTypeProperties(flowTypesPath, propertyPath => {
43+
setPropDescriptor(documentation, propertyPath)
44+
});
5445
}

src/utils/__tests__/getMethodDocumentation-test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ jest.disableAutomock();
1414

1515
describe('getMethodDocumentation', () => {
1616
let getMethodDocumentation;
17-
let expression, statement;
17+
let statement;
1818

1919
beforeEach(() => {
2020
getMethodDocumentation = require('../getMethodDocumentation').default;
21-
({expression, statement} = require('../../../tests/utils'));
21+
({ statement } = require('../../../tests/utils'));
2222
});
2323

2424
describe('name', () => {

src/utils/__tests__/getTypeAnnotation-test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,12 @@
1313
jest.disableAutomock();
1414

1515
describe('getTypeAnnotation', () => {
16-
var expression, statement;
16+
var expression;
1717
var getTypeAnnotation;
1818

1919
beforeEach(() => {
2020
getTypeAnnotation = require('../getTypeAnnotation').default;
21-
({expression, statement} = require('../../../tests/utils'));
21+
({expression} = require('../../../tests/utils'));
2222
});
2323

2424
it('detects simple type', () => {

src/utils/getFlowTypeFromReactComponent.js

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
*
1111
*/
1212

13-
import getPropertyName from './getPropertyName';
1413
import getTypeAnnotation from '../utils/getTypeAnnotation';
1514
import getMemberValuePath from '../utils/getMemberValuePath';
1615
import isReactComponentClass from '../utils/isReactComponentClass';
@@ -51,7 +50,48 @@ export default (path: NodePath): ?NodePath => {
5150
if (typePath && types.GenericTypeAnnotation.check(typePath.node)) {
5251
typePath = resolveToValue(typePath.get('id'));
5352
if (
54-
!typePath ||
53+
!typePath ||
54+
types.Identifier.check(typePath.node) ||
55+
isUnreachableFlowType(typePath)
56+
) {
57+
return;
58+
}
59+
60+
typePath = typePath.get('right');
61+
}
62+
63+
return typePath;
64+
}
65+
66+
export function applyToFlowTypeProperties(
67+
path: NodePath,
68+
callback: (propertyPath: NodePath) => void
69+
) {
70+
if (path.node.properties) {
71+
path.get('properties').each(
72+
propertyPath => callback(propertyPath)
73+
);
74+
} else if (path.node.type === 'IntersectionTypeAnnotation') {
75+
path.get('types').each(
76+
typesPath => applyToFlowTypeProperties(typesPath, callback)
77+
);
78+
} else if (path.node.type !== 'UnionTypeAnnotation') {
79+
// The react-docgen output format does not currently allow
80+
// for the expression of union types
81+
let typePath = resolveGenericTypeAnnotation(path);
82+
if (typePath) {
83+
applyToFlowTypeProperties(typePath, callback);
84+
}
85+
}
86+
}
87+
88+
function resolveGenericTypeAnnotation(path: NodePath): ?NodePath {
89+
// If the node doesn't have types or properties, try to get the type.
90+
let typePath: ?NodePath;
91+
if (path && types.GenericTypeAnnotation.check(path.node)) {
92+
typePath = resolveToValue(path.get('id'));
93+
if (
94+
!typePath ||
5595
types.Identifier.check(typePath.node) ||
5696
isUnreachableFlowType(typePath)
5797
) {

0 commit comments

Comments
 (0)