Skip to content

Commit

Permalink
Fix display-name for stateless components (fixes #256)
Browse files Browse the repository at this point in the history
  • Loading branch information
yannickcr committed Oct 19, 2015
1 parent b094b99 commit 15f0925
Show file tree
Hide file tree
Showing 4 changed files with 235 additions and 28 deletions.
48 changes: 44 additions & 4 deletions lib/rules/display-name.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ module.exports = function(context) {
* @returns {Boolean} True ifcomponent have a name, false if not.
*/
function hasTranspilerName(node) {
var namedAssignment = (
var namedObjectAssignment = (
node.type === 'ObjectExpression' &&
node.parent &&
node.parent.parent &&
Expand All @@ -85,7 +85,7 @@ module.exports = function(context) {
node.parent.parent.left.property.name !== 'exports'
)
);
var namedDeclaration = (
var namedObjectDeclaration = (
node.type === 'ObjectExpression' &&
node.parent &&
node.parent.parent &&
Expand All @@ -96,7 +96,23 @@ module.exports = function(context) {
node.id && node.id.name
);

if (namedAssignment || namedDeclaration || namedClass) {
var namedFunctionDeclaration = (
(node.type === 'FunctionDeclaration' || node.type === 'FunctionExpression') &&
node.id &&
node.id.name
);

var namedFunctionExpression = (
(node.type === 'FunctionExpression' || node.type === 'ArrowFunctionExpression') &&
node.parent &&
(node.parent.type === 'VariableDeclarator' || node.parent.method === true)
);

if (
namedObjectAssignment || namedObjectDeclaration ||
namedClass ||
namedFunctionDeclaration || namedFunctionExpression
) {
return true;
}
return false;
Expand All @@ -119,13 +135,37 @@ module.exports = function(context) {
if (!isDisplayNameDeclaration(node.property)) {
return;
}
var component = componentList.getByName(node.object.name);
var component = componentList.getByName(context.getSource(node.object));
if (!component) {
return;
}
markDisplayNameAsDeclared(component.node);
},

FunctionExpression: function(node) {
componentList.set(context, node);
if (!acceptTranspilerName || !hasTranspilerName(node)) {
return;
}
markDisplayNameAsDeclared(node);
},

FunctionDeclaration: function(node) {
componentList.set(context, node);
if (!acceptTranspilerName || !hasTranspilerName(node)) {
return;
}
markDisplayNameAsDeclared(node);
},

ArrowFunctionExpression: function(node) {
componentList.set(context, node);
if (!acceptTranspilerName || !hasTranspilerName(node)) {
return;
}
markDisplayNameAsDeclared(node);
},

MethodDefinition: function(node) {
if (!isDisplayNameDeclaration(node.key)) {
return;
Expand Down
42 changes: 33 additions & 9 deletions lib/util/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,17 @@ function isStatelessFunctionComponent(context, node) {
* @returns {Object} The component identifiers.
*/
function getIdentifiers(node) {
var name = node.id && node.id.name || DEFAULT_COMPONENT_NAME;
var name = [];
var loopNode = node;
var namePart = [];
while (loopNode) {
namePart = (loopNode.id && loopNode.id.name) || (loopNode.key && loopNode.key.name);
if (namePart) {
name.unshift(namePart);
}
loopNode = loopNode.parent;
}
name = name.join('.') || DEFAULT_COMPONENT_NAME;
var id = name + ':' + node.loc.start.line + ':' + node.loc.start.column;

return {
Expand All @@ -98,7 +108,7 @@ function getNode(context, node, list) {
}
// Node is already in the component list
var identifiers = getIdentifiers(ancestors[i]);
if (list && list[identifiers.id]) {
if (list && list[identifiers.id] && list[identifiers.id].isComponent) {
return ancestors[i];
}
}
Expand Down Expand Up @@ -136,7 +146,7 @@ List.prototype.getByNode = function(context, node) {
}
var identifiers = getIdentifiers(componentNode);

return this._list[identifiers.id] || null;
return this._list[identifiers.id] && this._list[identifiers.id].isComponent ? this._list[identifiers.id] : null;
};

/**
Expand All @@ -146,7 +156,11 @@ List.prototype.getByNode = function(context, node) {
*/
List.prototype.getByName = function(name) {
for (var component in this._list) {
if (this._list.hasOwnProperty(component) && this._list[component].name === name) {
if (
this._list.hasOwnProperty(component) &&
this._list[component].name === name &&
this._list[component].isComponent
) {
return this._list[component];
}
}
Expand All @@ -158,7 +172,14 @@ List.prototype.getByName = function(name) {
* @returns {Object} The component list.
*/
List.prototype.getList = function() {
return this._list;
var components = {};
for (var component in this._list) {
if (!this._list.hasOwnProperty(component) || this._list[component].isComponent !== true) {
continue;
}
components[component] = this._list[component];
}
return components;
};

/**
Expand All @@ -170,15 +191,18 @@ List.prototype.getList = function() {
*/
List.prototype.set = function(context, node, customProperties) {
var componentNode = getNode(context, node, this._list);
if (!componentNode) {
return null;
var isComponent = false;
if (componentNode) {
node = componentNode;
isComponent = true;
}

var identifiers = getIdentifiers(componentNode);
var identifiers = getIdentifiers(node);

var component = util._extend({
name: identifiers.name,
node: componentNode
node: node,
isComponent: isComponent
}, customProperties || {});

if (!this._list[identifiers.id]) {
Expand Down
161 changes: 152 additions & 9 deletions tests/lib/rules/display-name.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,124 @@ ruleTester.run('display-name', rule, {
'}'
].join('\n'),
parser: 'babel-eslint'
}, {
code: [
'var Hello = function() {',
' return <div>Hello {this.props.name}</div>;',
'}'
].join('\n'),
parser: 'babel-eslint',
options: [{
acceptTranspilerName: true
}]
}, {
code: [
'function Hello() {',
' return <div>Hello {this.props.name}</div>;',
'}'
].join('\n'),
parser: 'babel-eslint',
options: [{
acceptTranspilerName: true
}]
}, {
code: [
'var Hello = () => {',
' return <div>Hello {this.props.name}</div>;',
'}'
].join('\n'),
parser: 'babel-eslint',
options: [{
acceptTranspilerName: true
}]
}, {
code: [
'module.exports = function Hello() {',
' return <div>Hello {this.props.name}</div>;',
'}'
].join('\n'),
parser: 'babel-eslint',
options: [{
acceptTranspilerName: true
}]
}, {
code: [
'function Hello() {',
' return <div>Hello {this.props.name}</div>;',
'}',
'Hello.displayName = \'Hello\';'
].join('\n'),
parser: 'babel-eslint'
}, {
code: [
'var Hello = () => {',
' return <div>Hello {this.props.name}</div>;',
'}',
'Hello.displayName = \'Hello\';'
].join('\n'),
parser: 'babel-eslint'
}, {
code: [
'var Hello = function() {',
' return <div>Hello {this.props.name}</div>;',
'}',
'Hello.displayName = \'Hello\';'
].join('\n'),
parser: 'babel-eslint'
}, {
code: [
'var Mixins = {',
' Greetings: {',
' Hello: function() {',
' return <div>Hello {this.props.name}</div>;',
' }',
' }',
'}',
'Mixins.Greetings.Hello.displayName = \'Hello\';'
].join('\n'),
parser: 'babel-eslint'
}, {
code: [
'var Hello = React.createClass({',
' render: function() {',
' return <div>{this._renderHello()}</div>;',
' },',
' _renderHello: function() {',
' return <span>Hello {this.props.name}</span>;',
' }',
'});'
].join('\n'),
parser: 'babel-eslint',
options: [{
acceptTranspilerName: true
}]
}, {
code: [
'var Hello = React.createClass({',
' displayName: \'Hello\',',
' render: function() {',
' return <div>{this._renderHello()}</div>;',
' },',
' _renderHello: function() {',
' return <span>Hello {this.props.name}</span>;',
' }',
'});'
].join('\n'),
parser: 'babel-eslint'
}, {
code: [
'const Mixin = {',
' Button() {',
' return (',
' <button />',
' );',
' }',
'};'
].join('\n'),
parser: 'babel-eslint',
options: [{
acceptTranspilerName: true
}]
}],

invalid: [{
Expand All @@ -192,7 +310,7 @@ ruleTester.run('display-name', rule, {
jsx: false
},
errors: [{
message: 'Component definition is missing display name'
message: 'Hello component definition is missing display name'
}]
}, {
code: [
Expand All @@ -206,7 +324,7 @@ ruleTester.run('display-name', rule, {
jsx: true
},
errors: [{
message: 'Component definition is missing display name'
message: 'Hello component definition is missing display name'
}]
}, {
code: [
Expand Down Expand Up @@ -242,37 +360,62 @@ ruleTester.run('display-name', rule, {
jsx: true
},
errors: [{
message: 'Component definition is missing display name'
message: 'HelloComponent component definition is missing display name'
}]
}, {
code: [
'var Hello = function() {',
'module.exports = () => {',
' return <div>Hello {this.props.name}</div>;',
'}'
].join('\n'),
parser: 'babel-eslint',
options: [{
acceptTranspilerName: true
}],
errors: [{
message: 'Component definition is missing display name'
}]
}, {
code: [
'function Hello() {',
'module.exports = function() {',
' return <div>Hello {this.props.name}</div>;',
'}'
].join('\n'),
parser: 'babel-eslint',
options: [{
acceptTranspilerName: true
}],
errors: [{
message: 'Component definition is missing display name'
}]
}, {
code: [
'var Hello = React.createClass({',
' _renderHello: function() {',
' return <span>Hello {this.props.name}</span>;',
' },',
' render: function() {',
' return <div>{this._renderHello()}</div>;',
' }',
'});'
].join('\n'),
parser: 'babel-eslint',
errors: [{
message: 'Hello component definition is missing display name'
}]
}, {
code: [
'var Hello = () => {',
' return <div>Hello {this.props.name}</div>;',
'}'
'const Mixin = {',
' Button() {',
' return (',
' <button />',
' );',
' }',
'};'
].join('\n'),
parser: 'babel-eslint',
errors: [{
message: 'Component definition is missing display name'
message: 'Mixin.Button component definition is missing display name'
}]
}
]});

0 comments on commit 15f0925

Please sign in to comment.