Skip to content

Commit

Permalink
[Fix] Update void-dom-elements-no-children createElement checks
Browse files Browse the repository at this point in the history
Merge pull request jsx-eslint#1080 from jomasti/issue-1073
  • Loading branch information
ljharb authored Feb 21, 2017
2 parents c45ab86 + 416deff commit 8148833
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 22 deletions.
14 changes: 10 additions & 4 deletions lib/rules/void-dom-elements-no-children.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
var find = require('array.prototype.find');
var has = require('has');

var Components = require('../util/Components');

// ------------------------------------------------------------------------------
// Helpers
// ------------------------------------------------------------------------------
Expand Down Expand Up @@ -55,7 +57,7 @@ module.exports = {
schema: []
},

create: function(context) {
create: Components.detect(function(context, components, utils) {
return {
JSXElement: function(node) {
var elementName = node.openingElement.name.name;
Expand Down Expand Up @@ -93,11 +95,11 @@ module.exports = {
},

CallExpression: function(node) {
if (node.callee.type !== 'MemberExpression') {
if (node.callee.type !== 'MemberExpression' && node.callee.type !== 'Identifier') {
return;
}

if (node.callee.property.name !== 'createElement') {
if (!utils.hasDestructuredReactCreateElement() && !utils.isReactCreateElement(node)) {
return;
}

Expand All @@ -109,6 +111,10 @@ module.exports = {
return;
}

if (args.length < 2) {
return;
}

var firstChild = args[2];
if (firstChild) {
// e.g. React.createElement('br', undefined, 'Foo')
Expand Down Expand Up @@ -137,5 +143,5 @@ module.exports = {
}
}
};
}
})
};
54 changes: 36 additions & 18 deletions lib/util/Components.js
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,40 @@ function componentRule(rule, context) {
return false;
},

/**
* Check if createElement is destructured from React import
*
* @returns {Boolean} True if createElement is destructured from React
*/
hasDestructuredReactCreateElement: function() {
var variables = variableUtil.variablesInScope(context);
var variable = variableUtil.getVariable(variables, 'createElement');
if (variable) {
var map = variable.scope.set;
if (map.has('React')) {
return true;
}
}
return false;
},

/**
* Checks to see if node is called within React.createElement
*
* @param {ASTNode} node The AST node being checked.
* @returns {Boolean} True if React.createElement called
*/
isReactCreateElement: function(node) {
return (
node &&
node.callee &&
node.callee.object &&
node.callee.object.name === 'React' &&
node.callee.property &&
node.callee.property.name === 'createElement'
);
},

/**
* Check if the node is returning JSX
*
Expand Down Expand Up @@ -256,25 +290,9 @@ function componentRule(rule, context) {
node[property] &&
node[property].type === 'JSXElement'
;
var destructuredReactCreateElement = function () {
var variables = variableUtil.variablesInScope(context);
var variable = variableUtil.getVariable(variables, 'createElement');
if (variable) {
var map = variable.scope.set;
if (map.has('React')) {
return true;
}
}
return false;
};
var returnsReactCreateElement =
destructuredReactCreateElement() ||
node[property] &&
node[property].callee &&
node[property].callee.object &&
node[property].callee.object.name === 'React' &&
node[property].callee.property &&
node[property].callee.property.name === 'createElement'
this.hasDestructuredReactCreateElement() ||
this.isReactCreateElement(node[property])
;

return Boolean(
Expand Down
52 changes: 52 additions & 0 deletions tests/lib/rules/void-dom-elements-no-children.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,28 @@ ruleTester.run('void-dom-elements-no-children', rule, {
{
code: 'React.createElement("div", { dangerouslySetInnerHTML: { __html: "Foo" } });',
parserOptions: parserOptions
}, {
code: 'document.createElement("img")',
parserOptions: parserOptions
}, {
code: 'React.createElement("img");',
parserOptions: parserOptions
}, {
code: [
'import React from "react";',
'const { createElement } = React;',
'createElement("div")'
].join('\n'),
parser: 'babel-eslint',
parserOptions: parserOptions
}, {
code: [
'import React from "react";',
'const { createElement } = React;',
'createElement("img")'
].join('\n'),
parser: 'babel-eslint',
parserOptions: parserOptions
}
],
invalid: [
Expand Down Expand Up @@ -91,6 +113,36 @@ ruleTester.run('void-dom-elements-no-children', rule, {
code: 'React.createElement("br", { dangerouslySetInnerHTML: { __html: "Foo" } });',
errors: [{message: errorMessage('br')}],
parserOptions: parserOptions
},
{
code: [
'import React from "react";',
'const createElement = React.createElement;',
'createElement("img", {}, "Foo");'
].join('\n'),
errors: [{message: errorMessage('img')}],
parser: 'babel-eslint',
parserOptions: parserOptions
},
{
code: [
'import React from "react";',
'const createElement = React.createElement;',
'createElement("img", { children: "Foo" });'
].join('\n'),
errors: [{message: errorMessage('img')}],
parser: 'babel-eslint',
parserOptions: parserOptions
},
{
code: [
'import React from "react";',
'const createElement = React.createElement;',
'createElement("img", { dangerouslySetInnerHTML: { __html: "Foo" } });'
].join('\n'),
errors: [{message: errorMessage('img')}],
parser: 'babel-eslint',
parserOptions: parserOptions
}
]
});

0 comments on commit 8148833

Please sign in to comment.