Skip to content

Stateless Components #28

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

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 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
77 changes: 77 additions & 0 deletions src/__tests__/main-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,81 @@ describe('main', () => {
`);
});

describe('Stateless Component definition: ArrowFunctionExpression', () => {
test(`
import React, {PropTypes} from "React";

/**
* Example component description
*/
let Component = props => <div />;
Component.displayName = 'ABC';
Component.defaultProps = {
foo: true
};

Component.propTypes = {
/**
* Example prop description
*/
foo: PropTypes.bool
};

export default Component;
`);
});

describe('Stateless Component definition: FunctionDeclaration', () => {
test(`
import React, {PropTypes} from "React";

/**
* Example component description
*/
function Component (props) {
return <div />;
}

Component.displayName = 'ABC';
Component.defaultProps = {
foo: true
};

Component.propTypes = {
/**
* Example prop description
*/
foo: PropTypes.bool
};

export default Component;
`);
});

describe('Stateless Component definition: FunctionExpression', () => {
test(`
import React, {PropTypes} from "React";

/**
* Example component description
*/
let Component = function(props) {
return React.createElement('div', null);
}

Component.displayName = 'ABC';
Component.defaultProps = {
foo: true
};

Component.propTypes = {
/**
* Example prop description
*/
foo: PropTypes.bool
};

export default Component;
`);
});
});
10 changes: 10 additions & 0 deletions src/handlers/__tests__/propTypeHandler-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,16 @@ describe('propTypeHandler', () => {
);
});

describe('stateless component', () => {
test(
propTypesSrc => template(`
var Component = (props) => <div />;
Component.propTypes = ${propTypesSrc};
`),
src => statement(src)
);
});

it('does not error if propTypes cannot be found', () => {
var definition = expression('{fooBar: 42}');
expect(() => propTypeHandler(documentation, definition))
Expand Down
42 changes: 42 additions & 0 deletions src/resolver/__tests__/findAllComponentDefinitions-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,46 @@ describe('findAllComponentDefinitions', () => {
});
});

describe('stateless components', () => {

it('finds stateless components', () => {
var source = `
import React from 'React';
let ComponentA = () => <div />;
function ComponentB () { return React.createElement('div', null); }
const ComponentC = function(props) { return <div>{props.children}</div>; };
var Obj = {
component() { if (true) { return <div />; } }
};
`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function for this isn’t quite recursive enough. While all of these resolve as expected, the following two cases do not resolve correctly yet:

const ComponentG = function(props) {
  var nested = {
    helpers: {
      comp() { return <div>{props.children}</div>; }
    }
  };
  return nested.helpers.comp();
};
const ComponentF = function(props) {
  var other = () => <div>{props.children}</div>;
  var helpers = {
    comp: other
  };
  return helpers.comp();
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This works like a champ.


var result = parse(source);
expect(Array.isArray(result)).toBe(true);
expect(result.length).toBe(4);
});

it('finds React.createElement, independent of the var name', () => {
var source = `
import AlphaBetters from 'react';
function ComponentA () { return AlphaBetters.createElement('div', null); }
function ComponentB () { return 7; }
`;

var result = parse(source);
expect(Array.isArray(result)).toBe(true);
expect(result.length).toBe(1);
});

it('does not process X.createClass of other modules', () => {
var source = `
import R from 'FakeReact';
const ComponentA = () => R.createElement('div', null);
`;

var result = parse(source);
expect(Array.isArray(result)).toBe(true);
expect(result.length).toBe(0);
});
});

});
162 changes: 162 additions & 0 deletions src/resolver/__tests__/findExportedComponentDefinition-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,40 @@ describe('findExportedComponentDefinition', () => {
});
});

describe('stateless components', () => {

it('finds stateless component with JSX', () => {
var source = `
var React = require("React");
var Component = () => <div />;
module.exports = Component;
`;

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

it('finds stateless components with React.createElement, independent of the var name', () => {
var source = `
var R = require("React");
var Component = () => R.createElement('div', {});
module.exports = Component;
`;

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

it('does not process X.createElement of other modules', () => {
var source = `
var R = require("NoReact");
var Component = () => R.createElement({});
module.exports = Component;
`;

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

});

describe('module.exports = <C>; / exports.foo = <C>;', () => {

describe('React.createClass', () => {
Expand Down Expand Up @@ -449,6 +483,75 @@ describe('findExportedComponentDefinition', () => {
});
});

describe.only('stateless components', () => {

it('finds named exports', () => {
var source = `
import React from 'React';
export var somethingElse = 42,
Component = () => <div />;
`;
var result = parse(source);
expect(result).toBeDefined();
expect(result.node.type).toBe('ArrowFunctionExpression');

source = `
import React from 'React';
export let Component = () => <div />,
somethingElse = 42;
`;
result = parse(source);
expect(result).toBeDefined();
expect(result.node.type).toBe('ArrowFunctionExpression');

source = `
import React from 'React';
export const something = 21,
Component = () => <div />,
somethingElse = 42;
`;
result = parse(source);
expect(result).toBeDefined();
expect(result.node.type).toBe('ArrowFunctionExpression');

source = `
import React from 'React';
export var somethingElse = function() {};
export let Component = () => <div />
`;
result = parse(source);
expect(result).toBeDefined();
expect(result.node.type).toBe('ArrowFunctionExpression');
});

it('errors if multiple components are exported', () => {
var source = `
import React from 'React';
export var ComponentA = () => <div />
export var ComponentB = () => <div />
`;
expect(() => parse(source)).toThrow();

source = `
import React from 'React';
export var ComponentA = () => <div />
var ComponentB = () => <div />
export {ComponentB};
`;
expect(() => parse(source)).toThrow();
});

it('accepts multiple definitions if only one is exported', () => {
var source = `
import React from 'React';
var ComponentA = class extends React.Component {}
export var ComponentB = function() { return <div />; };
`;
var result = parse(source);
expect(result).toBeDefined();
expect(result.node.type).toBe('FunctionExpression');
});
});
});

describe('export {<C>};', () => {
Expand Down Expand Up @@ -566,6 +669,65 @@ describe('findExportedComponentDefinition', () => {

});

describe('stateless components', () => {

it('finds exported specifiers', () => {
var source = `
import React from 'React';
var foo = 42;
function Component = () { return <div />; }
export {foo, Component};
`;
var result = parse(source);
expect(result).toBeDefined();
expect(result.node.type).toBe('ClassExpression');

source = `
import React from 'React';
var foo = 42;
var Component = () => <div />;
export {Component, foo};
`;
result = parse(source);
expect(result).toBeDefined();
expect(result.node.type).toBe('ClassExpression');

source = `
import React from 'React';
var foo = 42;
var baz = 21;
var Component = function () { return <div />; }
export {foo, Component as bar, baz};
`;
result = parse(source);
expect(result).toBeDefined();
expect(result.node.type).toBe('ClassExpression');
});

it('errors if multiple components are exported', () => {
var source = `
import React from 'React';
var ComponentA = () => <div />;
function ComponentB() { return <div />; }
export {ComponentA as foo, ComponentB};
`;

expect(() => parse(source)).toThrow();
});

it('accepts multiple definitions if only one is exported', () => {
var source = `
import React from 'React';
var ComponentA = () => <div />;
var ComponentB = () => <div />;
export {ComponentA};
`;
var result = parse(source);
expect(result).toBeDefined();
expect(result.node.type).toBe('ArrowFunctionExpression');
});

});
});

// Only applies to classes
Expand Down
13 changes: 13 additions & 0 deletions src/resolver/findAllComponentDefinitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import isReactComponentClass from '../utils/isReactComponentClass';
import isReactCreateClassCall from '../utils/isReactCreateClassCall';
import isStatelessComponent from '../utils/isStatelessComponent';
import normalizeClassDefinition from '../utils/normalizeClassDefinition';
import resolveToValue from '../utils/resolveToValue';

Expand All @@ -34,7 +35,19 @@ export default function findAllReactCreateClassCalls(
return false;
}

function statelessVisitor(path) {
if (isStatelessComponent(path)) {
// TODO: normalizeStatelessDefinition to pick up propTypes
Copy link
Member

Choose a reason for hiding this comment

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

This is done right? (because we use a different way to resolve this information)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why yes I do believe it is!

definitions.push(path);
}
return false;
}

recast.visit(ast, {
visitFunctionDeclaration: statelessVisitor,
visitFunctionExpression: statelessVisitor,
visitProperty: statelessVisitor,
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need this, do we? What case does this capture which isn't captured otherwise?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this as part of supporting some of the permutations also being added to gaearon/babel-plugin-react-transform; namely this format: https://github.com/gaearon/babel-plugin-react-transform/pull/34/files#diff-9d6f6ff3fa6b5b63c09190bd7a2efb95R15

Honestly I’m not sure if that is captured or handled correctly yet. 😇

Copy link
Member

Choose a reason for hiding this comment

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

I think this should be handled by the other visitors since methods are just represented as function expressions in the AST.

On Oct 7, 2015, at 12:51 PM, Dustan Kasten <[email protected]mailto:[email protected]> wrote:

In src/resolver/findAllComponentDefinitions.jshttps://github.com//pull/28#discussion_r41437429:

recast.visit(ast, {

  • visitFunctionDeclaration: statelessVisitor,
  • visitFunctionExpression: statelessVisitor,
  • visitProperty: statelessVisitor,

I added this as part of supporting some of the permutations also being added to gaearon/babel-plugin-react-transform; namely this format: https://github.com/gaearon/babel-plugin-react-transform/pull/34/files#diff-9d6f6ff3fa6b5b63c09190bd7a2efb95R15

Honestly I’m not sure if that is captured or handled correctly yet. [:innocent:]


Reply to this email directly or view it on GitHubhttps://github.com//pull/28/files#r41437429.

visitArrowFunctionExpression: statelessVisitor,
visitClassExpression: classVisitor,
visitClassDeclaration: classVisitor,
visitCallExpression: function(path) {
Expand Down
7 changes: 5 additions & 2 deletions src/resolver/findExportedComponentDefinition.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import isExportsOrModuleAssignment from '../utils/isExportsOrModuleAssignment';
import isReactComponentClass from '../utils/isReactComponentClass';
import isReactCreateClassCall from '../utils/isReactCreateClassCall';
import isStatelessComponent from '../utils/isStatelessComponent';
import normalizeClassDefinition from '../utils/normalizeClassDefinition';
import resolveExportDeclaration from '../utils/resolveExportDeclaration';
import resolveToValue from '../utils/resolveToValue';
Expand All @@ -24,7 +25,7 @@ function ignore() {
}

function isComponentDefinition(path) {
return isReactCreateClassCall(path) || isReactComponentClass(path);
return isReactCreateClassCall(path) || isReactComponentClass(path) || isStatelessComponent(path);
}

function resolveDefinition(definition, types) {
Expand All @@ -37,6 +38,8 @@ function resolveDefinition(definition, types) {
} else if(isReactComponentClass(definition)) {
normalizeClassDefinition(definition);
return definition;
} else if (isStatelessComponent(definition)) {
return definition;
}
return null;
}
Expand All @@ -51,7 +54,7 @@ function resolveDefinition(definition, types) {
* If a definition is part of the following statements, it is considered to be
* exported:
*
* modules.exports = Defintion;
* modules.exports = Definition;
* exports.foo = Definition;
* export default Definition;
* export var Definition = ...;
Expand Down
Loading