Skip to content

Commit

Permalink
Refactor for flow intersection and union Props
Browse files Browse the repository at this point in the history
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
  • Loading branch information
palange committed Jul 18, 2016
1 parent cb96efb commit 1203552
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 33 deletions.
1 change: 1 addition & 0 deletions flow/react-docgen.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ type FlowTypeDescriptor = {
elements?: Array<FlowTypeDescriptor>,
type?: 'object' | 'function',
signature?: flowObjectSignatureType | flowFunctionSignatureType,
alias?: string,
};

type PropDescriptor = {
Expand Down
46 changes: 46 additions & 0 deletions src/handlers/__tests__/flowTypeDocBlockHandler-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,52 @@ describe('flowTypeDocBlockHandler', () => {
.not.toThrow();
});

it('supports intersection proptypes', () => {
var definition = statement(`
(props: Props) => <div />;
var React = require('React');
import type Imported from 'something';
type BarProps = {
/** bar description */
bar: 'barValue'
}
type Props = Imported & BarProps & {
/** foo description */
foo: 'fooValue'
};
`).get('expression');

flowTypeDocBlockHandler(documentation, definition);

expect(documentation.descriptors).toEqual({
foo: {
description: 'foo description',
},
bar: {
description: 'bar description',
},
});

it('does not support union proptypes', () => {
var definition = statement(`
(props: Props) => <div />;
var React = require('React');
import type Imported from 'something';
type Other = { bar: 'barValue' };
type Props = Imported | Other | { foo: 'fooValue' };
`).get('expression');

expect(() => flowTypeDocBlockHandler(documentation, definition))
.toThrowError(TypeError, "react-docgen doesn't support Props of union types");
});
});


describe('does not error for unreachable type', () => {
function test(code) {
var definition = statement(code).get('expression');
Expand Down
22 changes: 8 additions & 14 deletions src/handlers/__tests__/flowTypeHandler-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,14 +170,13 @@ describe('flowTypeHandler', () => {
it('supports intersection proptypes', () => {
var definition = statement(`
(props: Props) => <div />;
var React = require('React');
import type Imported from 'something';
type Props = Imported & { foo: 'bar' };
`).get('expression');


flowTypeHandler(documentation, definition);

expect(documentation.descriptors).toEqual({
Expand All @@ -188,24 +187,19 @@ describe('flowTypeHandler', () => {
});
});

it('supports union proptypes', () => {
it('does not support union proptypes', () => {
var definition = statement(`
(props: Props) => <div />;
var React = require('React');
import type Imported from 'something';
type Props = Imported | { foo: 'bar' };
type Other = { bar: 'barValue' };
type Props = Imported | Other | { foo: 'fooValue' };
`).get('expression');

flowTypeHandler(documentation, definition);

expect(documentation.descriptors).toEqual({
foo: {
flowType: {},
required: true,
},
});
expect(() => flowTypeHandler(documentation, definition))
.toThrowError(TypeError, "react-docgen doesn't support Props of union types");
});

describe('does not error for unreachable type', () => {
Expand Down
10 changes: 7 additions & 3 deletions src/handlers/flowTypeDocBlockHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,23 @@

import type Documentation from '../Documentation';
import setPropDescription from '../utils/setPropDescription';
import getFlowTypeFromReactComponent from '../utils/getFlowTypeFromReactComponent';
import getFlowTypeFromReactComponent, {
applyToFlowTypeProperties,
} from '../utils/getFlowTypeFromReactComponent';

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

if (!flowTypesPath) {
return;
}

flowTypesPath.get('properties').each(propertyPath => setPropDescription(documentation, propertyPath));
applyToFlowTypeProperties(flowTypesPath, propertyPath =>
setPropDescription(documentation, propertyPath)
);
}
28 changes: 13 additions & 15 deletions src/handlers/flowTypeHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,30 +14,26 @@ import type Documentation from '../Documentation';

import getFlowType from '../utils/getFlowType';
import getPropertyName from '../utils/getPropertyName';
import getFlowTypeFromReactComponent from '../utils/getFlowTypeFromReactComponent';
import getFlowTypeFromReactComponent, {
applyToFlowTypeProperties,
} from '../utils/getFlowTypeFromReactComponent';
import isUnreachableFlowType from '../utils/isUnreachableFlowType';
import recast from 'recast';
import resolveToValue from '../utils/resolveToValue';
import { getDocblock } from '../utils/docblock';


var {types: {namedTypes: types}} = recast;

function setPropDescriptor(documentation: Documentation, path: NodePath): void {
const propDescriptor = documentation.getPropDescriptor(getPropertyName(path));
const type = getFlowType(path.get('value'));

if (type) {
propDescriptor.flowType = type;
propDescriptor.required = !path.node.optional;
}
}

function findAndSetTypes(documentation: Documentation, path: NodePath): void {
if (path.node.properties) {
path.get('properties').each(
propertyPath => setPropDescriptor(documentation, propertyPath)
);
} else if (path.node.types) {
path.get('types').each(
typesPath => findAndSetTypes(documentation, typesPath)
);
}
}

/**
* This handler tries to find flow Type annotated react components and extract
* its types to the documentation. It also extracts docblock comments which are
Expand All @@ -50,5 +46,7 @@ export default function flowTypeHandler(documentation: Documentation, path: Node
return;
}

findAndSetTypes(documentation, flowTypesPath);
applyToFlowTypeProperties(flowTypesPath, propertyPath => {
setPropDescriptor(documentation, propertyPath)
});
}
45 changes: 44 additions & 1 deletion src/utils/getFlowTypeFromReactComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,50 @@ export default (path: NodePath): ?NodePath => {
if (typePath && types.GenericTypeAnnotation.check(typePath.node)) {
typePath = resolveToValue(typePath.get('id'));
if (
!typePath ||
!typePath ||
types.Identifier.check(typePath.node) ||
isUnreachableFlowType(typePath)
) {
return;
}

typePath = typePath.get('right');
}

return typePath;
}

export function applyToFlowTypeProperties(
path: NodePath,
callback: (propertyPath: NodePath) => void
) {
if (path.node.properties) {
path.get('properties').each(
propertyPath => callback(propertyPath)
);
} else if (path.node.type == 'IntersectionTypeAnnotation') {
path.get('types').each(
typesPath => applyToFlowTypeProperties(typesPath, callback)
);
} else if (path.node.type == 'UnionTypeAnnotation') {
// The react-docgen output format does not currently allow
// for the expression of union types
throw new TypeError("react-docgen doesn't support Props of union types");
} else {
let typePath = resolveGenericTypeAnnotation(path);
if (typePath) {
applyToFlowTypeProperties(typePath, callback);
}
}
}

function resolveGenericTypeAnnotation(path: NodePath): ?NodePath {
// If the node doesn't have types or properties, try to get the type.
let typePath: ?NodePath;
if (path && types.GenericTypeAnnotation.check(path.node)) {
typePath = resolveToValue(path.get('id'));
if (
!typePath ||
types.Identifier.check(typePath.node) ||
isUnreachableFlowType(typePath)
) {
Expand Down

0 comments on commit 1203552

Please sign in to comment.