Skip to content

refactor: JSX fragments related rules no longer rely on 'jsxPragma' and 'jsxPragmaFrag' in 'react-x' settings #893

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

Merged
merged 2 commits into from
Dec 30, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions .markdownlint.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"extends": "markdownlint/style/all",
"MD013": false,
"MD024": false,
"MD033": false,
"MD041": false
}
686 changes: 347 additions & 339 deletions CHANGELOG.md

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion packages/core/docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@
- [isDeclaredInRenderPropLoose](functions/isDeclaredInRenderPropLoose.md)
- [isForwardRef](functions/isForwardRef.md)
- [isForwardRefCall](functions/isForwardRefCall.md)
- [isFragmentElement](functions/isFragmentElement.md)
- [isFromReact](functions/isFromReact.md)
- [isFunctionOfRenderMethod](functions/isFunctionOfRenderMethod.md)
- [isInitializedFromReact](functions/isInitializedFromReact.md)
Expand Down
31 changes: 0 additions & 31 deletions packages/core/docs/functions/isFragmentElement.md

This file was deleted.

4 changes: 4 additions & 0 deletions packages/core/docs/functions/isInitializedFromReact.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,8 @@ The identifier that’s used for JSX Element creation.

`"createElement"`

**Deprecated**

#### jsxPragmaFrag

`string`
Expand All @@ -180,6 +182,8 @@ This should not be a member expression (i.e. use "Fragment" instead of "React.Fr

`"Fragment"`

**Deprecated**

#### polymorphicPropName

`string`
Expand Down
1 change: 0 additions & 1 deletion packages/core/src/element/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
export * from "./get-element-represent-name";
export * from "./hierarchy";
export * from "./misc";
28 changes: 0 additions & 28 deletions packages/core/src/element/misc.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,9 @@ ruleTester.run(RULE_NAME, rule, {
/* tsx */ `<>{cloneElement(children, { ref: childrenRef })}</>`,
{
code: /* tsx */ `
<React.SomeFragment>
<SomeReact.SomeFragment>
{<Foo />}
</React.SomeFragment>
</SomeReact.SomeFragment>
`,
settings: {
"react-x": {
Expand All @@ -208,14 +208,6 @@ ruleTester.run(RULE_NAME, rule, {
},
},
},
{
code: /* tsx */ `<NotReact.Fragment />`,
settings: {
"react-x": {
strictImportCheck: true,
},
},
},
{
code: /* tsx */ `{foo}`,
options: [{ allowExpressions: false }],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import * as AST from "@eslint-react/ast";
import { isFragmentElement } from "@eslint-react/core";
import * as JSX from "@eslint-react/jsx";
import type { RuleContext, RuleFeature } from "@eslint-react/types";
import { AST_NODE_TYPES } from "@typescript-eslint/types";
import type { TSESTree } from "@typescript-eslint/utils";
import { isMatching, P } from "ts-pattern";

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

Expand All @@ -19,48 +17,62 @@ export type MessageID =
| "noUselessFragment"
| "noUselessFragmentInBuiltIn";

// eslint-disable-next-line @typescript-eslint/consistent-return
function check(
type Options = [
{
allowExpressions: boolean;
},
];

const defaultOptions = [{
allowExpressions: true,
}] as const satisfies Options;

function checkAndReport(
node: TSESTree.JSXElement | TSESTree.JSXFragment,
context: RuleContext,
allowExpressions: boolean,
) {
const initialScope = context.sourceCode.getScope(node);
// return if the fragment is keyed (e.g. <Fragment key={key}>)
if (JSX.isKeyedElement(node, initialScope)) return;
// report if the fragment is placed inside a built-in component (e.g. <div><></></div>)
if (JSX.isBuiltInElement(node.parent)) context.report({ messageId: "noUselessFragmentInBuiltIn", node });
// report and return if the fragment has no children (e.g. <></>)
if (node.children.length === 0) return context.report({ messageId: "noUselessFragment", node });
const isChildren = AST.isOneOf([AST_NODE_TYPES.JSXElement, AST_NODE_TYPES.JSXFragment])(node.parent);
const [firstChildren] = node.children;
// <Foo content={<>ee eeee eeee ...</>} />
if (allowExpressions && node.children.length === 1 && JSX.isLiteral(firstChildren) && !isChildren) return;
if (!allowExpressions && isChildren) {
const isChildElement = AST.isOneOf([AST_NODE_TYPES.JSXElement, AST_NODE_TYPES.JSXFragment])(node.parent);
switch (true) {
// <Foo content={<>ee eeee eeee ...</>} />
case allowExpressions
&& !isChildElement
&& node.children.length === 1
&& JSX.isLiteral(node.children.at(0)): {
return;
}
// <Foo><>hello, world</></Foo>
return context.report({ messageId: "noUselessFragment", node });
} else if (!allowExpressions && !isChildren && node.children.length === 1) {
// const foo = <>{children}</>;
// return <>{children}</>;
return context.report({ messageId: "noUselessFragment", node });
case !allowExpressions
&& isChildElement: {
return context.report({ messageId: "noUselessFragment", node });
}
case !allowExpressions
&& !isChildElement
&& node.children.length === 1: {
// const foo = <>{children}</>;
// return <>{children}</>;
return context.report({ messageId: "noUselessFragment", node });
}
}
const nonPaddingChildren = node.children.filter((child) => !JSX.isPaddingSpaces(child));
if (nonPaddingChildren.length > 1) return;
if (nonPaddingChildren.length === 0) return context.report({ messageId: "noUselessFragment", node });
const [first] = nonPaddingChildren;
if (
isMatching({ type: AST_NODE_TYPES.JSXExpressionContainer, expression: P.not(AST_NODE_TYPES.CallExpression) }, first)
) return;
context.report({ messageId: "noUselessFragment", node });
const firstNonPaddingChild = nonPaddingChildren.at(0);
switch (true) {
case nonPaddingChildren.length === 0:
case nonPaddingChildren.length === 1
&& firstNonPaddingChild?.type !== AST_NODE_TYPES.JSXExpressionContainer: {
return context.report({ messageId: "noUselessFragment", node });
}
}
return;
}

type Options = [
{
allowExpressions: boolean;
},
];

const defaultOptions = [{
allowExpressions: true,
}] as const satisfies Options;

export default createRule<Options, MessageID>({
meta: {
type: "problem",
Expand Down Expand Up @@ -88,11 +100,11 @@ export default createRule<Options, MessageID>({
const { allowExpressions = true } = option;
return {
JSXElement(node) {
if (!isFragmentElement(node, context)) return;
check(node, context, allowExpressions);
if (JSX.getElementName(node.openingElement).split(".").at(-1) !== "Fragment") return;
checkAndReport(node, context, allowExpressions);
},
JSXFragment(node) {
check(node, context, allowExpressions);
checkAndReport(node, context, allowExpressions);
},
};
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,5 @@ ruleTester.run(RULE_NAME, rule, {
"<Fragment key={item.id}>{item.value}</Fragment>",
"<Fooo content={<>eeee ee eeeeeee eeeeeeee</>} />",
"<>{foos.map(foo => foo)}</>",
{
code: /* tsx */ `<NotReact.Fragment />`,
settings: {
"react-x": {
strictImportCheck: true,
},
},
},
],
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { isFragmentElement } from "@eslint-react/core";
import { F, O } from "@eslint-react/eff";
import * as JSX from "@eslint-react/jsx";
import type { RuleFeature } from "@eslint-react/types";
import type { TSESTree } from "@typescript-eslint/types";
import type { ReportDescriptor } from "@typescript-eslint/utils/ts-eslint";
Expand Down Expand Up @@ -30,7 +30,7 @@ export default createRule<[], MessageID>({
name: RULE_NAME,
create(context) {
function getReportDescriptor(node: TSESTree.JSXElement): O.Option<ReportDescriptor<MessageID>> {
if (!isFragmentElement(node, context)) return O.none();
if (JSX.getElementName(node.openingElement).split(".").at(-1) !== "Fragment") return O.none();
const hasAttributes = node.openingElement.attributes.length > 0;
if (hasAttributes) return O.none();
return O.some({
Expand Down
12 changes: 10 additions & 2 deletions packages/shared/docs/functions/defineSettings.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ The identifier that’s used for JSX Element creation.

`"createElement"`

**Deprecated**

#### jsxPragmaFrag

`string` = `...`
Expand All @@ -168,6 +170,8 @@ This should not be a member expression (i.e. use "Fragment" instead of "React.Fr

`"Fragment"`

**Deprecated**

#### polymorphicPropName

`string` = `...`
Expand Down Expand Up @@ -335,7 +339,7 @@ This allows to specify a custom import location for React when not using the off
`"@pika/react"`
```

### jsxPragma?
### ~~jsxPragma?~~

> `optional` **jsxPragma**: `string`

Expand All @@ -345,7 +349,9 @@ The identifier that’s used for JSX Element creation.

`"createElement"`

### jsxPragmaFrag?
#### Deprecated

### ~~jsxPragmaFrag?~~

> `optional` **jsxPragmaFrag**: `string`

Expand All @@ -359,6 +365,8 @@ This should not be a member expression (i.e. use "Fragment" instead of "React.Fr

`"Fragment"`

#### Deprecated

### polymorphicPropName?

> `optional` **polymorphicPropName**: `string`
Expand Down
2 changes: 2 additions & 0 deletions packages/shared/src/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,14 @@ export const ESLintReactSettingsSchema = object({
/**
* The identifier that’s used for JSX Element creation.
* @default `"createElement"`
* @deprecated
*/
jsxPragma: optional(string()),
/**
* The identifier that’s used for JSX fragment elements.
* @description This should not be a member expression (i.e. use "Fragment" instead of "React.Fragment").
* @default `"Fragment"`
* @deprecated
*/
jsxPragmaFrag: optional(string()),
/**
Expand Down
2 changes: 0 additions & 2 deletions website/pages/docs/advanced-configuration.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ export default [
settings: {
"react-x": {
importSource: "react",
jsxPragma: "createElement",
jsxPragmaFrag: "Fragment",
additionalHooks: {
useLayoutEffect: ["useIsomorphicLayoutEffect"],
},
Expand Down
17 changes: 0 additions & 17 deletions website/pages/docs/configurations.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,6 @@ This allows to specify a custom import location for React when not using the off

(e.g. `@pika/react`, etc)

### `jsxPragma`

(type: `string`, default: `"createElement"`)

The identifier that's used for JSX Element creation.\
This should not be a member expression (i.e. use `"createElement"` instead of `"React.createElement"`).

### `jsxPragmaFrag`

(type: `string`, default: `"Fragment"`)

The identifier that's used for JSX fragment elements.\
This should not be a member expression (i.e. use `"Fragment"` instead of `"React.Fragment"`).

### `version`

(type: `string`, default: `"detect"`)
Expand Down Expand Up @@ -82,9 +68,6 @@ export default [
...eslintReact.configs.recommended,
settings: {
"react-x": {
importSource: "react",
jsxPragma: "createElement",
jsxPragmaFrag: "Fragment",
additionalHooks: {
useLayoutEffect: ["useIsomorphicLayoutEffect"],
},
Expand Down
Loading