Skip to content

Commit 7e293da

Browse files
CompuIvesfkling
authored andcommitted
Fix issue with computed props for functional components (#132)
Fixes issue #131. The issue was that props that didn't have any defaults were still evaluated (since the prop was indeed referenced) and received { value: propName, computed: 'true' } as defaultProp. I also found an issue that imported values were not properly recognised by defaultprops for stateless components.
1 parent 766b8bc commit 7e293da

File tree

2 files changed

+37
-5
lines changed

2 files changed

+37
-5
lines changed

src/handlers/__tests__/defaultPropsHandler-test.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,5 +185,33 @@ describe('defaultPropsHandler', () => {
185185
`;
186186
test(parse(src).get('body', 0, 'expression'));
187187
});
188+
189+
it('should find prop default values that are imported variables', () => {
190+
var src = `
191+
import ImportedComponent from './ImportedComponent';
192+
193+
({
194+
foo = ImportedComponent,
195+
}) => <div />
196+
`;
197+
defaultPropsHandler(documentation, parse(src).get('body', 1, 'expression'));
198+
199+
expect(documentation.descriptors).toEqual({
200+
foo: {
201+
defaultValue: {
202+
value: 'ImportedComponent',
203+
computed: true,
204+
},
205+
},
206+
});
207+
});
208+
209+
it('should work with no defaults', () => {
210+
var src = `
211+
({ foo }) => <div />
212+
`;
213+
defaultPropsHandler(documentation, parse(src).get('body', 0, 'expression'));
214+
expect(documentation.descriptors).toEqual({});
215+
});
188216
});
189217
});

src/handlers/defaultPropsHandler.js

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,15 +86,19 @@ function getDefaultPropsPath(componentDefinition: NodePath): ?NodePath {
8686

8787
function getDefaultValuesFromProps(
8888
properties: NodePath,
89-
documentation: Documentation
89+
documentation: Documentation,
90+
isStatelessComponent: boolean,
9091
) {
9192
properties
9293
.filter(propertyPath => types.Property.check(propertyPath.node))
94+
// Don't evaluate property if component is functional and the node is not an AssignmentPattern
95+
.filter(propertyPath => !isStatelessComponent || types.AssignmentPattern.check(propertyPath.get('value').node))
9396
.forEach(function(propertyPath) {
9497
var propDescriptor = documentation.getPropDescriptor(
9598
getPropertyName(propertyPath)
9699
);
97-
var defaultValue = getDefaultValue(propertyPath.get('value'));
100+
var value = isStatelessComponent ? propertyPath.get('value', 'right') : propertyPath.get('value');
101+
var defaultValue = getDefaultValue(value, isStatelessComponent);
98102
if (defaultValue) {
99103
propDescriptor.defaultValue = defaultValue;
100104
}
@@ -105,17 +109,17 @@ export default function defaultPropsHandler(
105109
documentation: Documentation,
106110
componentDefinition: NodePath
107111
) {
108-
109112
var statelessProps = null;
110113
var defaultPropsPath = getDefaultPropsPath(componentDefinition);
111114
if (isStatelessComponent(componentDefinition)) {
112115
statelessProps = getStatelessPropsPath(componentDefinition);
113116
}
114117

118+
// Do both statelessProps and defaultProps if both are available so defaultProps can override
115119
if (statelessProps && types.ObjectPattern.check(statelessProps.node)) {
116-
getDefaultValuesFromProps(statelessProps.get('properties'), documentation);
120+
getDefaultValuesFromProps(statelessProps.get('properties'), documentation, true);
117121
}
118122
if (defaultPropsPath && types.ObjectExpression.check(defaultPropsPath.node)) {
119-
getDefaultValuesFromProps(defaultPropsPath.get('properties'), documentation);
123+
getDefaultValuesFromProps(defaultPropsPath.get('properties'), documentation, false);
120124
}
121125
}

0 commit comments

Comments
 (0)