Skip to content

Commit

Permalink
feat: add no-children-prop, closes #101 (#132)
Browse files Browse the repository at this point in the history
  • Loading branch information
Rel1cx authored Nov 12, 2023
1 parent f553ba3 commit 00bcb7f
Show file tree
Hide file tree
Showing 10 changed files with 271 additions and 4 deletions.
2 changes: 2 additions & 0 deletions packages/eslint-plugin-react/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import noChildrenForEach from "./rules/no-children-for-each";
import noChildrenInVoidDomElements from "./rules/no-children-in-void-dom-elements";
import noChildrenMap from "./rules/no-children-map";
import noChildrenOnly from "./rules/no-children-only";
import noChildrenProp from "./rules/no-children-prop";
import noChildrenToArray from "./rules/no-children-to-array";
import noClassComponent from "./rules/no-class-component";
import noCloneElement from "./rules/no-clone-element";
Expand Down Expand Up @@ -35,6 +36,7 @@ export const rules = {
"no-children-in-void-dom-elements": noChildrenInVoidDomElements,
"no-children-map": noChildrenMap,
"no-children-only": noChildrenOnly,
"no-children-prop": noChildrenProp,
"no-children-to-array": noChildrenToArray,
"no-class-component": noClassComponent,
"no-clone-element": noCloneElement,
Expand Down
46 changes: 46 additions & 0 deletions packages/eslint-plugin-react/src/rules/no-children-prop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# react/no-children-prop

<!-- end auto-generated rule header -->

## Rule category

Suspicious.

## What it does

Disallows passing of children as props.

## Why is this bad?

Most of the time, `children` should be actual `children`, not passed in as a `prop`.

When using JSX, the `children` should be nested between the opening and closing tags. When not using JSX, the `children` should be passed as additional arguments to `React.createElement`.

## Examples

### ❌ Incorrect

```tsx
<div children='Children' />

<Component children={<AnotherComponent />} />
<Component children={['Child 1', 'Child 2']} />

React.createElement("div", { children: 'Children' })
```

### ✅ Correct

```tsx
<div>Children</div>

<Component>Children</Component>

<Component>
<span>Child 1</span>
<span>Child 2</span>
</Component>

React.createElement("div", {}, 'Children')
React.createElement("div", 'Child 1', 'Child 2')
```
125 changes: 125 additions & 0 deletions packages/eslint-plugin-react/src/rules/no-children-prop.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
import { allValid } from "@eslint-react/shared";
import dedent from "dedent";

import RuleTester, { getFixturesRootDir } from "../../../../test/rule-tester";
import rule, { RULE_NAME } from "./no-children-prop";

const rootDir = getFixturesRootDir();

const ruleTester = new RuleTester({
parser: "@typescript-eslint/parser",
parserOptions: {
ecmaFeatures: {
jsx: true,
},
ecmaVersion: 2021,
sourceType: "module",
project: "./tsconfig.json",
tsconfigRootDir: rootDir,
},
});

ruleTester.run(RULE_NAME, rule, {
valid: [
...allValid,
"<div />;",
"<div></div>;",
'React.createElement("div", {});',
'React.createElement("div", undefined);',
'<div className="class-name"></div>;',
'React.createElement("div", {className: "class-name"});',
"<div>Children</div>;",
'React.createElement("div", "Children");',
'React.createElement("div", {}, "Children");',
'React.createElement("div", undefined, "Children");',
'<div className="class-name">Children</div>;',
'React.createElement("div", {className: "class-name"}, "Children");',
"<div><div /></div>;",
'React.createElement("div", React.createElement("div"));',
'React.createElement("div", {}, React.createElement("div"));',
'React.createElement("div", undefined, React.createElement("div"));',
"<div><div /><div /></div>;",
'React.createElement("div", React.createElement("div"), React.createElement("div"));',
'React.createElement("div", {}, React.createElement("div"), React.createElement("div"));',
'React.createElement("div", undefined, React.createElement("div"), React.createElement("div"));',
'React.createElement("div", [React.createElement("div"), React.createElement("div")]);',
'React.createElement("div", {}, [React.createElement("div"), React.createElement("div")]);',
'React.createElement("div", undefined, [React.createElement("div"), React.createElement("div")]);',
"<MyComponent />",
"React.createElement(MyComponent);",
"React.createElement(MyComponent, {});",
"React.createElement(MyComponent, undefined);",
"<MyComponent>Children</MyComponent>;",
'React.createElement(MyComponent, "Children");',
'React.createElement(MyComponent, {}, "Children");',
'React.createElement(MyComponent, undefined, "Children");',
'<MyComponent className="class-name"></MyComponent>;',
'React.createElement(MyComponent, {className: "class-name"});',
'<MyComponent className="class-name">Children</MyComponent>;',
'React.createElement(MyComponent, {className: "class-name"}, "Children");',
'<MyComponent className="class-name" {...props} />;',
'React.createElement(MyComponent, {className: "class-name", ...props});',
],
invalid: [
{
code: "<div children />;", // not a valid use case but make sure we don't crash
errors: [{ messageId: "NO_CHILDREN_PROP" }],
},
{
code: '<div children="Children" />;',
errors: [{ messageId: "NO_CHILDREN_PROP" }],
},
{
code: "<div children={<div />} />;",
errors: [{ messageId: "NO_CHILDREN_PROP" }],
},
{
code: "<div children={[<div />, <div />]} />;",
errors: [{ messageId: "NO_CHILDREN_PROP" }],
},
{
code: '<div children="Children">Children</div>;',
errors: [{ messageId: "NO_CHILDREN_PROP" }],
},
{
code: 'React.createElement("div", {children: "Children"});',
errors: [{ messageId: "NO_CHILDREN_PROP" }],
},
{
code: 'React.createElement("div", {children: "Children"}, "Children");',
errors: [{ messageId: "NO_CHILDREN_PROP" }],
},
{
code: 'React.createElement("div", {children: React.createElement("div")});',
errors: [{ messageId: "NO_CHILDREN_PROP" }],
},
{
code: 'React.createElement("div", {children: [React.createElement("div"), React.createElement("div")]});',
errors: [{ messageId: "NO_CHILDREN_PROP" }],
},
{
code: '<MyComponent children="Children" />',
errors: [{ messageId: "NO_CHILDREN_PROP" }],
},
{
code: 'React.createElement(MyComponent, {children: "Children"});',
errors: [{ messageId: "NO_CHILDREN_PROP" }],
},
{
code: '<MyComponent className="class-name" children="Children" />;',
errors: [{ messageId: "NO_CHILDREN_PROP" }],
},
{
code: 'React.createElement(MyComponent, {children: "Children", className: "class-name"});',
errors: [{ messageId: "NO_CHILDREN_PROP" }],
},
{
code: '<MyComponent {...props} children="Children" />;',
errors: [{ messageId: "NO_CHILDREN_PROP" }],
},
{
code: 'React.createElement(MyComponent, {...props, children: "Children"})',
errors: [{ messageId: "NO_CHILDREN_PROP" }],
},
],
});
91 changes: 91 additions & 0 deletions packages/eslint-plugin-react/src/rules/no-children-prop.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { NodeType } from "@eslint-react/ast";
import { findPropInProperties, getProp, isCreateElementCall } from "@eslint-react/jsx";
import { O } from "@eslint-react/tools";
import type { ESLintUtils } from "@typescript-eslint/utils";
import type { ConstantCase } from "string-ts";

import { createRule } from "../utils";

export const RULE_NAME = "no-children-prop";

export type MessageID = ConstantCase<typeof RULE_NAME>;

// No need to check because TypeScript don't allow this
// function isValidAttribute(
// prop: TSESTree.JSXAttribute | TSESTree.JSXSpreadAttribute,
// ) {
// return (
// "value" in prop
// && prop.value
// && "expression" in prop.value
// && isFunction(prop.value.expression)
// );
// }

// No need to check because TypeScript don't allow this
// function isValidProperty(
// prop:
// | TSESTree.PropertyComputedName
// | TSESTree.PropertyNonComputedName
// | TSESTree.RestElement
// | TSESTree.SpreadElement,
// ) {
// return (
// "value" in prop
// && prop.value
// && "type" in prop.value
// && isFunction(prop.value)
// );
// }

export default createRule<[], MessageID>({
name: RULE_NAME,
meta: {
type: "suggestion",
docs: {
description: "disallow passing of children as props",
recommended: "recommended",
requiresTypeChecking: false,
},
schema: [],
messages: {
NO_CHILDREN_PROP: "Children should always be actual children, not passed in as a prop.",
},
},
defaultOptions: [],
create(context) {
return {
JSXElement(node) {
O.map(getProp(node.openingElement.attributes, "children", context), prop => {
context.report({
messageId: "NO_CHILDREN_PROP",
node: prop,
});
});
},
// eslint-disable-next-line perfectionist/sort-objects
CallExpression(node) {
if (node.arguments.length === 0) {
return;
}

if (!isCreateElementCall(node, context)) {
return;
}

const [_, props] = node.arguments;

if (!props || props.type !== NodeType.ObjectExpression) {
return;
}

O.map(findPropInProperties(props.properties, context)("children"), prop => {
context.report({
messageId: "NO_CHILDREN_PROP",
node: prop,
});
});
},
};
},
});
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { NodeType } from "@eslint-react/ast";
import { getPropValue } from "@eslint-react/jsx";
import { findPropInAttributes, getPropValue } from "@eslint-react/jsx";
import { F, O } from "@eslint-react/tools";
import { ESLintUtils } from "@typescript-eslint/utils";
import { getStaticValue } from "@typescript-eslint/utils/ast-utils";
import { isString } from "effect/Predicate";
import type { ConstantCase } from "string-ts";

import { findPropInAttributes } from "../../../jsx/src/prop/find-prop-in-attributes";
import { createRule } from "../utils";

export const RULE_NAME = "no-unsafe-target-blank";
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const rulePreset = {
"react/no-children-in-void-dom-elements": "warn",
"react/no-children-map": "warn",
"react/no-children-only": "warn",
"react/no-children-prop": "warn",
"react/no-children-to-array": "warn",
"react/no-class-component": "warn",
"react/no-clone-element": "warn",
Expand Down Expand Up @@ -73,6 +74,7 @@ const recommendedPreset = {
"naming-convention/component-name": "warn",

"react/no-children-in-void-dom-elements": "warn",
"react/no-children-prop": "warn",
"react/no-class-component": "warn",
"react/no-clone-element": "warn",
"react/no-constructed-context-value": "error",
Expand Down
2 changes: 1 addition & 1 deletion packages/jsx/docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ A function that searches for a property in the given properties

| Name | Type |
| :--------- | :------------------------------------------------------------ |
| `props` | `JSXAttribute`[] |
| `props` | (`JSXAttribute` \| `JSXSpreadAttribute`)[] |
| `propName` | `string` |
| `context` | `Readonly`\<`RuleContext`\<`string`, readonly `unknown`[]\>\> |

Expand Down
2 changes: 1 addition & 1 deletion packages/jsx/src/prop/get-prop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { ESLintUtils } from "@typescript-eslint/utils";
import { findPropInAttributes } from "./find-prop-in-attributes";

export function getProp(
props: TSESTree.JSXAttribute[],
props: (TSESTree.JSXAttribute | TSESTree.JSXSpreadAttribute)[],
propName: string,
context: RuleContext,
): O.Option<TSESTree.JSXAttribute | TSESTree.JSXSpreadAttribute> {
Expand Down
1 change: 1 addition & 0 deletions website/pages/rules/_meta.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"react-no-children-in-void-dom-elements": "react/no-children-in-void-dom-elements",
"react-no-children-map": "react/no-children-map",
"react-no-children-only": "react/no-children-only",
"react-no-children-prop": "react/no-children-prop",
"react-no-children-to-array": "react/no-children-to-array",
"react-no-class-component": "react/no-class-component",
"react-no-clone-element": "react/no-clone-element",
Expand Down
1 change: 1 addition & 0 deletions website/pages/rules/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
| [react/no-children-in-void-dom-elements](react-no-children-in-void-dom-elements) | disallow passing children to void DOM elements |
| [react/no-children-map](react-no-children-map) | disallow `Children.map` |
| [react/no-children-only](react-no-children-only) | disallow `Children.only()` |
| [react/no-children-prop](react-no-children-prop) | disallow passing of children as props |
| [react/no-children-to-array](react-no-children-to-array) | disallow `Children.toArray()` |
| [react/no-class-component](react-no-class-component) | enforce that there are no class components |
| [react/no-clone-element](react-no-clone-element) | disallow `cloneElement` |
Expand Down

0 comments on commit 00bcb7f

Please sign in to comment.