Skip to content
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

Check JSXFragment in most places where checking JSXElement, fixes #106. #122

Merged
merged 1 commit into from
Dec 30, 2023
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
14 changes: 14 additions & 0 deletions docs/prefer-for.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,20 @@ let Component = (props) => (
</ol>
);

let Component = (props) => (
<>
{props.data.map((d) => (
<li>{d.text}</li>
))}
</>
);
// after eslint --fix:
let Component = (props) => (
<>
<For each={props.data}>{(d) => <li>{d.text}</li>}</For>
</>
);

let Component = (props) => (
<ol>
{props.data.map((d) => (
Expand Down
14 changes: 14 additions & 0 deletions docs/prefer-show.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,20 @@ function Component(props) {
);
}

function Component(props) {
return <>{props.cond && <span>Content</span>}</>;
}
// after eslint --fix:
function Component(props) {
return (
<>
<Show when={props.cond}>
<span>Content</span>
</Show>
</>
);
}

function Component(props) {
return <div>{props.cond ? <span>Content</span> : <span>Fallback</span>}</div>;
}
Expand Down
4 changes: 2 additions & 2 deletions src/rules/prefer-for.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { TSESTree as T, ESLintUtils, ASTUtils } from "@typescript-eslint/utils";
import { isFunctionNode } from "../utils";
import { isFunctionNode, isJSXElementOrFragment } from "../utils";

const createRule = ESLintUtils.RuleCreator.withoutDocs;
const { getPropertyName } = ASTUtils;
Expand Down Expand Up @@ -55,7 +55,7 @@ export default createRule({
const callOrChain = node.parent?.type === "ChainExpression" ? node.parent : node;
if (
callOrChain.parent?.type === "JSXExpressionContainer" &&
callOrChain.parent.parent?.type === "JSXElement"
isJSXElementOrFragment(callOrChain.parent.parent)
) {
// check for Array.prototype.map in JSX
if (
Expand Down
9 changes: 5 additions & 4 deletions src/rules/prefer-show.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { TSESTree as T, ESLintUtils } from "@typescript-eslint/utils";
import { isJSXElementOrFragment } from "../utils";

const createRule = ESLintUtils.RuleCreator.withoutDocs;

Expand All @@ -25,7 +26,7 @@ export default createRule({
const sourceCode = context.getSourceCode();
const putIntoJSX = (node: T.Node): string => {
const text = sourceCode.getText(node);
return node.type === "JSXElement" || node.type === "JSXFragment" ? text : `{${text}}`;
return isJSXElementOrFragment(node) ? text : `{${text}}`;
};

const logicalExpressionHandler = (node: T.LogicalExpression) => {
Expand All @@ -36,7 +37,7 @@ export default createRule({
fix: (fixer) =>
fixer.replaceText(
node.parent?.type === "JSXExpressionContainer" &&
node.parent.parent?.type === "JSXElement"
isJSXElementOrFragment(node.parent.parent)
? node.parent
: node,
`<Show when={${sourceCode.getText(node.left)}}>${putIntoJSX(node.right)}</Show>`
Expand All @@ -55,7 +56,7 @@ export default createRule({
fix: (fixer) =>
fixer.replaceText(
node.parent?.type === "JSXExpressionContainer" &&
node.parent.parent?.type === "JSXElement"
isJSXElementOrFragment(node.parent.parent)
? node.parent
: node,
`<Show when={${sourceCode.getText(node.test)}} fallback={${sourceCode.getText(
Expand All @@ -68,7 +69,7 @@ export default createRule({

return {
JSXExpressionContainer(node) {
if (node.parent?.type !== "JSXElement") {
if (!isJSXElementOrFragment(node.parent)) {
return;
}
if (node.expression.type === "LogicalExpression") {
Expand Down
5 changes: 3 additions & 2 deletions src/rules/reactivity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
isDOMElementName,
ignoreTransparentWrappers,
getFunctionName,
isJSXElementOrFragment,
} from "../utils";

const { findVariable, getFunctionHeadLocation } = ASTUtils;
Expand Down Expand Up @@ -527,7 +528,7 @@ export default createRule<Options, MessageIds>({
if (
// The signal is not being called and is being used as a props.children, where calling
// the signal was the likely intent.
elementOrAttribute?.type === "JSXElement" ||
isJSXElementOrFragment(elementOrAttribute) ||
// We can't say for sure about user components, but we know for a fact that a signal
// should not be passed to a non-event handler DOM element attribute without calling it.
(elementOrAttribute?.type === "JSXAttribute" &&
Expand Down Expand Up @@ -886,7 +887,7 @@ export default createRule<Options, MessageIds>({
// to the DOM. This is semantically a "called function", so it's fine to read reactive
// variables here.
pushTrackedScope(node.expression, "called-function");
} else if (node.parent?.type === "JSXElement" && isFunctionNode(node.expression)) {
} else if (isJSXElementOrFragment(node.parent) && isFunctionNode(node.expression)) {
pushTrackedScope(node.expression, "function"); // functions inline in JSX containers will be tracked
} else {
pushTrackedScope(node.expression, "expression");
Expand Down
5 changes: 5 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ export const isProgramOrFunctionNode = (
node: T.Node | null | undefined
): node is ProgramOrFunctionNode => !!node && PROGRAM_OR_FUNCTION_TYPES.includes(node.type);

export const isJSXElementOrFragment = (
node: T.Node | null | undefined
): node is T.JSXElement | T.JSXFragment =>
node?.type === "JSXElement" || node?.type === "JSXFragment";

export const getFunctionName = (node: FunctionNode): string | null => {
if (
(node.type === "FunctionDeclaration" || node.type === "FunctionExpression") &&
Expand Down
5 changes: 5 additions & 0 deletions test/rules/prefer-for.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ export const cases = run("prefer-for", rule, {
errors: [{ messageId: "preferFor" }],
output: `let Component = (props) => <ol><For each={props.data}>{d => <li>{d.text}</li>}</For></ol>;`,
},
{
code: `let Component = (props) => <>{props.data.map(d => <li>{d.text}</li>)}</>;`,
errors: [{ messageId: "preferFor" }],
output: `let Component = (props) => <><For each={props.data}>{d => <li>{d.text}</li>}</For></>;`,
},
{
code: `let Component = (props) => <ol>{props.data.map(d => <li key={d.id}>{d.text}</li>)}</ol>;`,
errors: [{ messageId: "preferFor" }],
Expand Down
11 changes: 11 additions & 0 deletions test/rules/prefer-show.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@ export const cases = run("prefer-show", rule, {
return <div><Show when={props.cond}><span>Content</span></Show></div>;
}`,
},
{
code: `
function Component(props) {
return <>{props.cond && <span>Content</span>}</>;
}`,
errors: [{ messageId: "preferShowAnd" }],
output: `
function Component(props) {
return <><Show when={props.cond}><span>Content</span></Show></>;
}`,
},
{
code: `
function Component(props) {
Expand Down