Skip to content

Commit d2790e5

Browse files
committed
refactor: minor improvements
1 parent c3fd18c commit d2790e5

File tree

7 files changed

+78
-110
lines changed

7 files changed

+78
-110
lines changed

packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.ts

Lines changed: 42 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export function create(context: RuleContext<MessageID, Options>, [option]: Optio
5757
const { allowExpressions = true } = option;
5858
return {
5959
JSXElement(node) {
60-
if (!JSX.isFragmentElement(node)) return;
60+
if (!JSX.isJsxFragmentElement(node)) return;
6161
checkNode(context, node, allowExpressions);
6262
},
6363
JSXFragment(node) {
@@ -66,13 +66,6 @@ export function create(context: RuleContext<MessageID, Options>, [option]: Optio
6666
};
6767
}
6868

69-
/**
70-
* Check if a node is a Literal or JSXText
71-
* @param node The AST node to check
72-
* @returns boolean `true` if the node is a Literal or JSXText
73-
*/
74-
const isLiteral = AST.isOneOf([T.Literal, T.JSXText]);
75-
7669
/**
7770
* Check if a Literal or JSXText node is whitespace
7871
* @param node The AST node to check
@@ -88,7 +81,7 @@ function isWhiteSpace(node: TSESTree.JSXText | TSESTree.Literal) {
8881
* @returns boolean
8982
*/
9083
function isPaddingSpaces(node: TSESTree.Node) {
91-
return isLiteral(node)
84+
return JSX.isJsxText(node)
9285
&& isWhiteSpace(node)
9386
&& node.raw.includes("\n");
9487
}
@@ -108,55 +101,20 @@ function checkNode(
108101
node: TSESTree.JSXElement | TSESTree.JSXFragment,
109102
allowExpressions: boolean,
110103
) {
111-
function fix(fixer: RuleFixer) {
112-
// Not safe to fix fragments without a jsx parent.
113-
if (node.parent.type !== T.JSXElement && node.parent.type !== T.JSXFragment) {
114-
// const a = <></>
115-
if (node.children.length === 0) {
116-
return null;
117-
}
118-
119-
// const a = <>cat {meow}</>
120-
if (
121-
node.children.some(
122-
(child) =>
123-
(isLiteral(child) && !isWhiteSpace(child))
124-
|| AST.is(T.JSXExpressionContainer)(child),
125-
)
126-
) {
127-
return null;
128-
}
129-
}
130-
131-
// Not safe to fix `<Eeee><>foo</></Eeee>` because `Eeee` might require its children be a ReactElement.
132-
if (JSX.isUserDefinedElement(node.parent)) {
133-
return null;
134-
}
135-
136-
const opener = node.type === T.JSXFragment ? node.openingFragment : node.openingElement;
137-
const closer = node.type === T.JSXFragment ? node.closingFragment : node.closingElement;
138-
139-
const childrenText = opener.type === T.JSXOpeningElement && opener.selfClosing
140-
? ""
141-
: context.sourceCode.getText().slice(opener.range[1], closer?.range[0]);
142-
143-
return fixer.replaceText(node, trimLikeReact(childrenText));
144-
}
145-
146104
const initialScope = context.sourceCode.getScope(node);
147105
// return if the fragment is keyed (e.g. <Fragment key={key}>)
148-
if (JSX.isKeyedElement(node, initialScope)) {
106+
if (JSX.isJsxKeyedElement(node, initialScope)) {
149107
return;
150108
}
151109
// report if the fragment is placed inside a built-in component (e.g. <div><></></div>)
152-
if (JSX.isBuiltInElement(node.parent)) {
110+
if (JSX.isJsxBuiltInElement(node.parent)) {
153111
context.report({
154112
messageId: "uselessFragment",
155113
node,
156114
data: {
157115
reason: "placed inside a built-in component",
158116
},
159-
fix,
117+
fix: getFix(context, node),
160118
});
161119
}
162120
// report and return if the fragment has no children (e.g. <></>)
@@ -167,7 +125,7 @@ function checkNode(
167125
data: {
168126
reason: "contains less than two children",
169127
},
170-
fix,
128+
fix: getFix(context, node),
171129
});
172130
return;
173131
}
@@ -177,7 +135,7 @@ function checkNode(
177135
case allowExpressions
178136
&& !isChildElement
179137
&& node.children.length === 1
180-
&& isLiteral(node.children.at(0)): {
138+
&& JSX.isJsxText(node.children.at(0)): {
181139
return;
182140
}
183141
// <Foo><>hello, world</></Foo>
@@ -189,7 +147,7 @@ function checkNode(
189147
data: {
190148
reason: "contains less than two children",
191149
},
192-
fix,
150+
fix: getFix(context, node),
193151
});
194152
return;
195153
}
@@ -204,7 +162,7 @@ function checkNode(
204162
data: {
205163
reason: "contains less than two children",
206164
},
207-
fix,
165+
fix: getFix(context, node),
208166
});
209167
return;
210168
}
@@ -221,10 +179,42 @@ function checkNode(
221179
data: {
222180
reason: "contains less than two children",
223181
},
224-
fix,
182+
fix: getFix(context, node),
225183
});
226184
return;
227185
}
228186
}
229187
return;
230188
}
189+
190+
function getFix(context: RuleContext, node: TSESTree.JSXElement | TSESTree.JSXFragment) {
191+
if (!canFix(node)) return null;
192+
return (fixer: RuleFixer) => {
193+
const opener = node.type === T.JSXFragment ? node.openingFragment : node.openingElement;
194+
const closer = node.type === T.JSXFragment ? node.closingFragment : node.closingElement;
195+
196+
const childrenText = opener.type === T.JSXOpeningElement && opener.selfClosing
197+
? ""
198+
: context.sourceCode.getText().slice(opener.range[1], closer?.range[0]);
199+
200+
return fixer.replaceText(node, trimLikeReact(childrenText));
201+
};
202+
}
203+
204+
function canFix(node: TSESTree.JSXElement | TSESTree.JSXFragment) {
205+
if (node.parent.type === T.JSXElement || node.parent.type === T.JSXFragment) {
206+
// Not safe to fix `<Eeee><>foo</></Eeee>` because `Eeee` might require its children be a ReactElement.
207+
return !JSX.isJsxUserDefinedElement(node.parent);
208+
}
209+
// Not safe to fix fragments without a jsx parent.
210+
// const a = <></>
211+
if (node.children.length === 0) {
212+
return false;
213+
}
214+
// dprint-ignore
215+
// const a = <>{meow}</>
216+
if (node.children.some((child) => (JSX.isJsxText(child) && !isWhiteSpace(child)) || AST.is(T.JSXExpressionContainer)(child))) {
217+
return false;
218+
}
219+
return true;
220+
}

packages/plugins/eslint-plugin-react-x/src/rules/prefer-shorthand-fragment.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export default createRule<[], MessageID>({
3535
export function create(context: RuleContext<MessageID, []>): RuleListener {
3636
return {
3737
JSXElement(node: TSESTree.JSXElement) {
38-
if (!JSX.isFragmentElement(node)) return;
38+
if (!JSX.isJsxFragmentElement(node)) return;
3939
const hasAttributes = node.openingElement.attributes.length > 0;
4040
if (hasAttributes) {
4141
return;

packages/utilities/jsx/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ export * from "./get-attribute-value";
55
export * from "./get-element-type";
66
export * from "./has-attribute";
77
export * from "./is-jsx";
8+
export * from "./is-jsx-element";
89
export * from "./is-jsx-fragment";
910
export * from "./is-jsx-text";
10-
export * from "./is-kind-of-element";
1111
export * from "./jsx-detection-hint";
1212
export * from "./to-string";
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import type { Scope } from "@typescript-eslint/scope-manager";
2+
import type { TSESTree } from "@typescript-eslint/types";
3+
import { AST_NODE_TYPES as T } from "@typescript-eslint/types";
4+
5+
import { getElementType } from "./get-element-type";
6+
import { hasAttribute } from "./has-attribute";
7+
8+
export function isJsxFragmentElement(node: TSESTree.Node) {
9+
if (node.type !== T.JSXElement) return false;
10+
return getElementType(node)
11+
.split(".")
12+
.at(-1) === "Fragment";
13+
}
14+
15+
export function isJsxKeyedElement(node: TSESTree.Node, initialScope?: Scope) {
16+
return node.type === T.JSXElement
17+
&& hasAttribute("key", node.openingElement.attributes, initialScope);
18+
}
19+
20+
export function isJsxBuiltInElement(node: TSESTree.Node) {
21+
return node.type === T.JSXElement
22+
&& node.openingElement.name.type === T.JSXIdentifier
23+
&& /^[a-z]/u.test(node.openingElement.name.name);
24+
}
25+
26+
export function isJsxUserDefinedElement(node: TSESTree.Node) {
27+
return node.type === T.JSXElement
28+
&& node.openingElement.name.type === T.JSXIdentifier
29+
&& /^[A-Z]/u.test(node.openingElement.name.name);
30+
}
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import type { _ } from "@eslint-react/eff";
22
import { AST_NODE_TYPES, type TSESTree } from "@typescript-eslint/types";
3-
import { isFragmentElement } from "./is-kind-of-element";
3+
import { isJsxFragmentElement } from "./is-jsx-element";
44

55
/**
66
* Check if a node is a `JSXFragment` or a `Fragment` element
77
* @param node The AST node to check
88
* @returns `true` if the node is a `JSXFragment` or a `Fragment` element
99
*/
10-
export function isJSXFragmentLike(node: TSESTree.Node | null | _) {
10+
export function isJsxFragment(node: TSESTree.Node | null | _) {
1111
if (node == null) return false;
12-
return node.type === AST_NODE_TYPES.JSXFragment || isFragmentElement(node);
12+
return node.type === AST_NODE_TYPES.JSXFragment || isJsxFragmentElement(node);
1313
}

packages/utilities/jsx/src/is-jsx-text.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { AST_NODE_TYPES as T, type TSESTree } from "@typescript-eslint/types";
66
* @param node The AST node to check
77
* @returns `true` if the node is a `JSXText` or a `Literal` node
88
*/
9-
export function isJSXTextLike(node: TSESTree.Node | null | _): node is TSESTree.JSXText | TSESTree.Literal {
9+
export function isJsxText(node: TSESTree.Node | null | _): node is TSESTree.JSXText | TSESTree.Literal {
1010
if (node == null) return false;
1111
return node.type === T.JSXText || node.type === T.Literal;
1212
}

packages/utilities/jsx/src/is-kind-of-element.ts

Lines changed: 0 additions & 52 deletions
This file was deleted.

0 commit comments

Comments
 (0)