Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for resolving exported components within HOCs #124

Merged
merged 1 commit into from
Sep 22, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions src/resolver/__tests__/findAllExportedComponentDefinitions-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,23 @@ describe('findAllExportedComponentDefinitions', () => {
expect(actual[1].node).toBe(expectedB.node);
});

it('finds multiple exported components with hocs', () => {
var parsed = parse(`
var R = require("React");
var ComponentA = R.createClass({});
var ComponentB = R.createClass({});
exports.ComponentA = hoc(ComponentA);
exports.ComponentB = hoc(ComponentB);
`);
var actual = findComponents(parsed);
var expectedA = parsed.get('body', 1, 'declarations', 0, 'init', 'arguments', 0);
var expectedB = parsed.get('body', 2, 'declarations', 0, 'init', 'arguments', 0);

expect(actual.length).toBe(2);
expect(actual[0].node).toBe(expectedA.node);
expect(actual[1].node).toBe(expectedB.node);
});

it('finds only exported components', () => {
var parsed = parse(`
var R = require("React");
Expand Down Expand Up @@ -700,6 +717,18 @@ describe('findAllExportedComponentDefinitions', () => {
expect(actual.length).toBe(2);
});

it('finds multiple components with hocs', () => {
var parsed = parse(`
var R = require("React");
var ComponentA = hoc(R.createClass({}));
var ComponentB = hoc(R.createClass({}));
export {ComponentA as foo, ComponentB};
`);
var actual = findComponents(parsed);

expect(actual.length).toBe(2);
});

it('finds only exported components', () => {
var parsed = parse(`
var R = require("React");
Expand Down Expand Up @@ -774,6 +803,20 @@ describe('findAllExportedComponentDefinitions', () => {
expect(actual.length).toBe(2);
});

it('finds multiple components with hocs', () => {
var parsed = parse(`
import React from 'React';
class ComponentA extends React.Component {};
class ComponentB extends React.Component {};
var WrappedA = hoc(ComponentA);
var WrappedB = hoc(ComponentB);
export {WrappedA, WrappedB};
`);
var actual = findComponents(parsed);

expect(actual.length).toBe(2);
});

it('finds only exported components', () => {
var parsed = parse(`
import React from 'React';
Expand Down
86 changes: 86 additions & 0 deletions src/resolver/__tests__/findExportedComponentDefinition-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,52 @@ describe('findExportedComponentDefinition', () => {
expect(parse(source)).toBeDefined();
});

it('finds React.createClass with hoc', () => {
var source = `
var React = require("React");
var Component = React.createClass({});
module.exports = hoc(Component);
`;

expect(parse(source)).toBeDefined();
});

it('finds React.createClass with hoc and args', () => {
var source = `
var React = require("React");
var Component = React.createClass({});
module.exports = hoc(arg1, arg2)(Component);
`;

expect(parse(source)).toBeDefined();
});

it('finds React.createClass with two hocs', () => {
var source = `
var React = require("React");
var Component = React.createClass({});
module.exports = hoc2(arg2b, arg2b)(
hoc1(arg1a, arg2a)(Component)
);
`;

expect(parse(source)).toBeDefined();
});

it('finds React.createClass with three hocs', () => {
var source = `
var React = require("React");
var Component = React.createClass({});
module.exports = hoc3(arg3a, arg3b)(
hoc2(arg2b, arg2b)(
hoc1(arg1a, arg2a)(Component)
)
);
`;

expect(parse(source)).toBeDefined();
});

it('finds React.createClass, independent of the var name', () => {
var source = `
var R = require("React");
Expand Down Expand Up @@ -317,6 +363,46 @@ describe('findExportedComponentDefinition', () => {
expect(result.node.type).toBe('ClassDeclaration');
});


it('finds default export with hoc', () => {
var source = `
import React from 'React';
class Component extends React.Component {}
export default hoc(Component);
`;

var result = parse(source);
expect(result).toBeDefined();
expect(result.node.type).toBe('ClassDeclaration');

});

it('finds default export with hoc and args', () => {
var source = `
import React from 'React';
class Component extends React.Component {}
export default hoc(arg1, arg2)(Component);
`;

var result = parse(source);
expect(result).toBeDefined();
expect(result.node.type).toBe('ClassDeclaration');
});

it('finds default export with two hocs', () => {
var source = `
import React from 'React';
class Component extends React.Component {}
export default hoc2(arg2b, arg2b)(
hoc1(arg1a, arg2a)(Component)
);
`;

var result = parse(source);
expect(result).toBeDefined();
expect(result.node.type).toBe('ClassDeclaration');
});

it('errors if multiple components are exported', () => {
var source = `
import React from 'React';
Expand Down
18 changes: 16 additions & 2 deletions src/resolver/findAllExportedComponentDefinitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import isStatelessComponent from '../utils/isStatelessComponent';
import normalizeClassDefinition from '../utils/normalizeClassDefinition';
import resolveExportDeclaration from '../utils/resolveExportDeclaration';
import resolveToValue from '../utils/resolveToValue';
import resolveHOC from '../utils/resolveHOC';

function ignore() {
return false;
Expand Down Expand Up @@ -65,7 +66,17 @@ export default function findExportedComponentDefinitions(

function exportDeclaration(path) {
var definitions: Array<?NodePath> = resolveExportDeclaration(path, types)
.filter(isComponentDefinition)
.reduce((acc, definition) => {
if (isComponentDefinition(definition)) {
acc.push(definition);
} else {
var resolved = resolveToValue(resolveHOC(definition));
if (isComponentDefinition(resolved)) {
acc.push(resolved);
}
}
return acc;
}, [])
.map((definition) => resolveDefinition(definition, types));

if (definitions.length === 0) {
Expand Down Expand Up @@ -107,7 +118,10 @@ export default function findExportedComponentDefinitions(
// expression, something like React.createClass
path = resolveToValue(path.get('right'));
if (!isComponentDefinition(path)) {
return false;
path = resolveToValue(resolveHOC(path));
if (!isComponentDefinition(path)) {
return false;
}
}
const definition = resolveDefinition(path, types);
if (definition && components.indexOf(definition) === -1) {
Expand Down
20 changes: 17 additions & 3 deletions src/resolver/findExportedComponentDefinition.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import isStatelessComponent from '../utils/isStatelessComponent';
import normalizeClassDefinition from '../utils/normalizeClassDefinition';
import resolveExportDeclaration from '../utils/resolveExportDeclaration';
import resolveToValue from '../utils/resolveToValue';
import resolveHOC from '../utils/resolveHOC';

var ERROR_MULTIPLE_DEFINITIONS =
'Multiple exported component definitions found.';
Expand All @@ -35,7 +36,7 @@ function resolveDefinition(definition, types) {
if (types.ObjectExpression.check(resolvedPath.node)) {
return resolvedPath;
}
} else if(isReactComponentClass(definition)) {
} else if (isReactComponentClass(definition)) {
normalizeClassDefinition(definition);
return definition;
} else if (isStatelessComponent(definition)) {
Expand Down Expand Up @@ -68,7 +69,17 @@ export default function findExportedComponentDefinition(

function exportDeclaration(path) {
var definitions = resolveExportDeclaration(path, types)
.filter(isComponentDefinition);
.reduce((acc, definition) => {
if (isComponentDefinition(definition)) {
acc.push(definition);
} else {
var resolved = resolveToValue(resolveHOC(definition));
if (isComponentDefinition(resolved)) {
acc.push(resolved);
}
}
return acc;
}, []);

if (definitions.length === 0) {
return false;
Expand Down Expand Up @@ -109,7 +120,10 @@ export default function findExportedComponentDefinition(
// expression, something like React.createClass
path = resolveToValue(path.get('right'));
if (!isComponentDefinition(path)) {
return false;
path = resolveToValue(resolveHOC(path));
if (!isComponentDefinition(path)) {
return false;
}
}
if (definition) {
// If a file exports multiple components, ... complain!
Expand Down
66 changes: 66 additions & 0 deletions src/utils/__tests__/resolveHOC-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* Copyright (c) 2015, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
*/

/*global jest, describe, beforeEach, it, expect*/

jest.disableAutomock();

describe('resolveHOC', () => {
var builders;
var utils;
var resolveHOC;

function parse(src) {
var root = utils.parse(src);
return root.get('body', root.node.body.length - 1, 'expression');
}

beforeEach(() => {
var recast = require('recast');
builders = recast.types.builders;
resolveHOC = require('../resolveHOC').default;
utils = require('../../../tests/utils');
});

it('resolves simple hoc', () => {
var path = parse([
'hoc(42);',
].join('\n'));
expect(resolveHOC(path).node).toEqualASTNode(builders.literal(42));
});

it('resolves simple hoc w/ multiple args', () => {
var path = parse([
'hoc1(arg1a, arg1b)(42);',
].join('\n'));
expect(resolveHOC(path).node).toEqualASTNode(builders.literal(42));
});

it('resolves nested hocs', () => {
var path = parse([
'hoc2(arg2b, arg2b)(',
' hoc1(arg1a, arg2a)(42)',
');',
].join('\n'));
expect(resolveHOC(path).node).toEqualASTNode(builders.literal(42));
});

it('resolves really nested hocs', () => {
var path = parse([
'hoc3(arg3a, arg3b)(',
' hoc2(arg2b, arg2b)(',
' hoc1(arg1a, arg2a)(42)',
' )',
');',
].join('\n'));
expect(resolveHOC(path).node).toEqualASTNode(builders.literal(42));
});

});
38 changes: 38 additions & 0 deletions src/utils/resolveHOC.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright (c) 2015, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @flow
*
*/

import recast from 'recast';
import isReactCreateClassCall from './isReactCreateClassCall';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about isReactComponentClass and isStatelessComponent?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React Classes and stateless functional components are not call expressions, but React.createClass() is.


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

/**
* If the path is a call expression, it recursively resolves to the
* rightmost argument, stopping if it finds a React.createClass call expression
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inspecting only the rightmost argument may not work for all cases.
One common case is for example Relay, where the component will be the first argument.

Also it might be interesting to investigate connect() of react-redux and recompose, as a baseline for example usage of HoCs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's interesting that Relay doesn't follow the convention I've seen for every other HOC, which is to take only one argument (the component) or have the component be the rightmost argument. Recompose follows this convention.

But as discussed in #80 you can easily turn any HOC into a HOC that takes a component as its only or rightmost argument.

*
* Else the path itself is returned.
*/
export default function resolveHOC(path: NodePath): NodePath {
var node = path.node;
if (types.CallExpression.check(node) && !isReactCreateClassCall(path)) {
if (node.arguments.length) {
return resolveHOC(path.get('arguments', node.arguments.length - 1));
}
}

return path;
}